fix(ng-model): Do not set value to an outdated value (#60)

* fix(ng-model): Do not set value to an outdated value * fix(ngModel): Add tests for ngModel * fix(lint): Fix lint error * fix(tests): Detach change detection ref * fix(lint): Fix lint error * chore(electron): Re-add line * fix(resize): Resize on initialization
Teradata · Jan 17, 2019 · 3aaf83e · 3aaf83e
1 parent ab69253
commit 3aaf83e
Show file tree

Hide file tree

Showing 2 changed files with 137 additions and 8 deletions.
diff --git a/src/platform/code-editor/code-editor.component.spec.ts b/src/platform/code-editor/code-editor.component.spec.ts
@@ -6,6 +6,7 @@ import {
 } from '@angular/core/testing';
 import { Component, ViewChild } from '@angular/core';
 import { TdCodeEditorComponent } from './';
+import { FormsModule } from '@angular/forms';
 
 declare var electron: any;
 declare const process: any;
@@ -26,8 +27,11 @@ describe('Component: App', () => {
         TdCodeEditorComponent,
         TestMultipleEditorsComponent,
         TestEditorOptionsComponent,
+        TestTwoWayBindingWithValueComponent,
+        TestTwoWayBindingWithNgModelComponent,
       ],
       imports: [
+        FormsModule,
       ],
     });
     TestBed.compileComponents();
@@ -293,6 +297,63 @@ describe('Component: App', () => {
     })();
   });
 
+  it('should work with 2 way binding via value', (done: DoneFn) => {
+    inject([], () => {
+      let fixture: ComponentFixture<any> = TestBed.createComponent(TestTwoWayBindingWithValueComponent);
+      let component: TestTwoWayBindingWithValueComponent = fixture.debugElement.componentInstance;
+      if (component.editor.isElectronApp) {
+        component.editor.setEditorNodeModuleDirOverride(electron.remote.process.env.NODE_MODULE_DIR);
+      }
+      const newSampleCode: string = 'const val = 2;';
+      component.sampleCode = newSampleCode;
+      fixture.changeDetectorRef.detectChanges();
+      fixture.detectChanges();
+      fixture.whenStable().then(() => {
+        component.editor.onEditorInitialized.subscribe(() => {
+          fixture.changeDetectorRef.detectChanges();
+          fixture.detectChanges();
+          fixture.whenStable().then(() => {
+            setTimeout(() => {
+              component.editor.getValue().subscribe((value: string) => {
+                expect(component.sampleCode).toBe(newSampleCode);
+                expect(value).toBe(newSampleCode);
+                done();
+              });
+            }, 1000); // wait for timeouts
+          });
+        });
+      });
+    })();
+  });
+
+  it('should work with 2 way binding via ngModel', (done: DoneFn) => {
+    inject([], () => {
+      let fixture: ComponentFixture<any> = TestBed.createComponent(TestTwoWayBindingWithNgModelComponent);
+      let component: TestTwoWayBindingWithNgModelComponent = fixture.debugElement.componentInstance;
+      if (component.editor.isElectronApp) {
+        component.editor.setEditorNodeModuleDirOverride(electron.remote.process.env.NODE_MODULE_DIR);
+      }
+      const newSampleCode: string = 'const val = 2;';
+      component.sampleCode = newSampleCode;
+      fixture.changeDetectorRef.detectChanges();
+      fixture.detectChanges();
+      fixture.whenStable().then(() => {
+        component.editor.onEditorInitialized.subscribe(() => {
+          fixture.changeDetectorRef.detectChanges();
+          fixture.detectChanges();
+          fixture.whenStable().then(() => {
+            setTimeout(() => {
+              component.editor.getValue().subscribe((value: string) => {
+                expect(component.sampleCode).toBe(newSampleCode);
+                expect(value).toBe(newSampleCode);
+                done();
+              });
+            }, 1000); // wait for timeouts
+          });
+        });
+      });
+    })();
+  });
 });
 
 @Component({
@@ -323,3 +384,39 @@ class TestMultipleEditorsComponent {
 class TestEditorOptionsComponent {
   @ViewChild('editor1') editor1: TdCodeEditorComponent;
 }
+
+@Component({
+  template: `
+    <div>
+      <td-code-editor
+        #editor
+        style="height: 200px"
+        language="javascript"
+        [(value)]="sampleCode"
+      >
+      </td-code-editor>
+    </div>
+  `,
+})
+class TestTwoWayBindingWithValueComponent {
+  @ViewChild('editor') editor: TdCodeEditorComponent;
+  sampleCode: string = `const val = 1;`;
+}
+
+@Component({
+  template: `
+    <div>
+      <td-code-editor
+        #editor
+        style="height: 200px"
+        language="javascript"
+        [(ngModel)]="sampleCode"
+      >
+      </td-code-editor>
+    </div>
+  `,
+})
+class TestTwoWayBindingWithNgModelComponent {
+  @ViewChild('editor') editor: TdCodeEditorComponent;
+  sampleCode: string = `const val = 1;`;
+}
diff --git a/src/platform/code-editor/code-editor.component.ts b/src/platform/code-editor/code-editor.component.ts
@@ -1,5 +1,5 @@
 import { Component, Input, Output, EventEmitter, OnInit, AfterViewInit,
-  ViewChild, ElementRef, forwardRef, NgZone, ChangeDetectorRef } from '@angular/core';
+  ViewChild, ElementRef, forwardRef, NgZone, ChangeDetectorRef, OnDestroy } from '@angular/core';
 import { NG_VALUE_ACCESSOR, ControlValueAccessor } from '@angular/forms';
 import { Observable, Subject } from 'rxjs';
 
@@ -27,7 +27,7 @@ declare const process: any;
     multi: true,
   }],
 })
-export class TdCodeEditorComponent implements OnInit, AfterViewInit, ControlValueAccessor {
+export class TdCodeEditorComponent implements OnInit, AfterViewInit, ControlValueAccessor, OnDestroy {
 
   private _editorStyle: string = 'width:100%;height:100%;border:1px solid grey;';
   private _appPath: string = '';
@@ -47,6 +47,8 @@ export class TdCodeEditorComponent implements OnInit, AfterViewInit, ControlValu
   private _editorOptions: any = {};
   private _isFullScreen: boolean = false;
   private _keycode: any;
+  private _setValueTimeout: any;
+  private initialContentChange: boolean = true;
 
   @ViewChild('editorContainer') _editorContainer: ElementRef;
 
@@ -103,6 +105,10 @@ export class TdCodeEditorComponent implements OnInit, AfterViewInit, ControlValu
    */
   @Input('value')
   set value(value: string) {
+    // Clear any timeout that might overwrite this value set in the future
+    if (this._setValueTimeout) {
+        clearTimeout(this._setValueTimeout);
+    }
     this._value = value;
     if (this._componentInitialized) {
         if (this._webview) {
@@ -117,7 +123,7 @@ export class TdCodeEditorComponent implements OnInit, AfterViewInit, ControlValu
                 this._fromEditor = false;
             } else {
                 // Editor is not loaded yet, try again in half a second
-                setTimeout(() => {
+                this._setValueTimeout = setTimeout(() => {
                     this.value = value;
                 }, 500);
             }
@@ -134,11 +140,15 @@ export class TdCodeEditorComponent implements OnInit, AfterViewInit, ControlValu
                 this.zone.run(() => this._value = value);
             } else {
               // Editor is not loaded yet, try again in half a second
-              setTimeout(() => {
+              this._setValueTimeout = setTimeout(() => {
                 this.value = value;
               }, 500);
             }
         }
+    } else {
+        this._setValueTimeout = setTimeout(() => {
+            this.value = value;
+        }, 500);
     }
   }
 
@@ -150,7 +160,11 @@ export class TdCodeEditorComponent implements OnInit, AfterViewInit, ControlValu
    * Implemented as part of ControlValueAccessor.
    */
   writeValue(value: any): void {
-    this.value = value;
+    // do not write if null or undefined
+    // tslint:disable-next-line
+    if ( value != undefined) {
+      this.value = value;
+    }
   }
   registerOnChange(fn: any): void {
     this.propagateChange = fn;
@@ -608,7 +622,12 @@ export class TdCodeEditorComponent implements OnInit, AfterViewInit, ControlValu
             } else if (event.channel === 'onEditorContentChange') {
               this._fromEditor = true;
               this.writeValue(event.args[0]);
+              if (this.initialContentChange) {
+                this.initialContentChange = false;
+                this.layout();
+            }
             } else if (event.channel === 'onEditorInitialized') {
+              this._componentInitialized = true;
               this._editorProxy = this.wrapEditorCalls(this._editor);
               this.onEditorInitialized.emit(this._editorProxy);
             } else if (event.channel === 'onEditorConfigurationChanged') {
@@ -621,7 +640,6 @@ export class TdCodeEditorComponent implements OnInit, AfterViewInit, ControlValu
         // append the webview to the DOM
         this._editorContainer.nativeElement.appendChild(this._webview);
     }
-    this._componentInitialized = true;
   }
 
   /**
@@ -666,6 +684,10 @@ export class TdCodeEditorComponent implements OnInit, AfterViewInit, ControlValu
     }
   }
 
+  ngOnDestroy(): void {
+    this._changeDetectorRef.detach();
+  }
+
   /**
    * waitForMonaco method Returns promise that will be fulfilled when monaco is available
    */
@@ -775,7 +797,12 @@ export class TdCodeEditorComponent implements OnInit, AfterViewInit, ControlValu
               let result: any = await origMethod.apply(that._editor, args);
               // since running javascript code manually need to force Angular to detect changes
               setTimeout(() => {
-                that.zone.run(() => that._changeDetectorRef.detectChanges());
+                that.zone.run(() => {
+                    // tslint:disable-next-line
+                    if (!that._changeDetectorRef['destroyed']) {
+                        that._changeDetectorRef.detectChanges();
+                    }
+                });
               });
               return result;
             }
@@ -801,16 +828,21 @@ export class TdCodeEditorComponent implements OnInit, AfterViewInit, ControlValu
     }, this.editorOptions));
     setTimeout(() => {
         this._editorProxy = this.wrapEditorCalls(this._editor);
+        this._componentInitialized = true;
         this.onEditorInitialized.emit(this._editorProxy);
     });
     this._editor.getModel().onDidChangeContent( (e: any) => {
         this._fromEditor = true;
         this.writeValue(this._editor.getValue());
+        if (this.initialContentChange) {
+            this.initialContentChange = false;
+            this.layout();
+        }
     });
     // need to manually resize the editor any time the window size
     // changes. See: https://github.com/Microsoft/monaco-editor/issues/28
     window.addEventListener('resize', () => {
-        this._editor.layout();
+        this.layout();
     });
     this.addFullScreenModeCommand();
   }