refactor(render): cleanup access to native dom elements

tbosch · tbosch · commit c8bdacb195bb · 2015-06-23T17:27:59.000-07:00
BREAKING CHANGES: - rename `ElementRef.domElement` to `ElementRef.nativeElement` - add `Renderer.getNativeElementSync` to make the app side less dependent on the dom renderer. - don’t use `ElementRef.nativeElement` in directives but use the methods on `Renderer` directly. - Removed `ElementRef.setAttribute`. Use `Renderer.setElementAttribute` instead. Closes angular#2712 Last part of angular#2476 Closes angular#2476
diff --git a/modules/angular2/debug.ts b/modules/angular2/debug.ts
@@ -1,2 +1,2 @@
 export * from './src/debug/debug_element';
-export {inspectDomElement, ELEMENT_PROBE_CONFIG} from './src/debug/debug_element_view_listener';
+export {inspectNativeElement, ELEMENT_PROBE_CONFIG} from './src/debug/debug_element_view_listener';
diff --git a/modules/angular2/src/core/application.ts b/modules/angular2/src/core/application.ts
@@ -56,7 +56,6 @@ import {AppViewListener} from 'angular2/src/core/compiler/view_listener';
 import {ProtoViewFactory} from 'angular2/src/core/compiler/proto_view_factory';
 import {Renderer, RenderCompiler} from 'angular2/src/render/api';
 import {DomRenderer, DOCUMENT_TOKEN} from 'angular2/src/render/dom/dom_renderer';
-import {resolveInternalDomView} from 'angular2/src/render/dom/view/view';
 import {DefaultDomCompiler} from 'angular2/src/render/dom/compiler/compiler';
 import {internalView} from 'angular2/src/core/compiler/view_ref';
 
@@ -85,10 +84,7 @@ function _injectorBindings(appComponentType): List<Type | Binding | List<any>> {
               // TODO(rado): investigate whether to support bindings on root component.
               return dynamicComponentLoader.loadAsRoot(appComponentType, null, injector)
                   .then((componentRef) => {
-                    var domView = resolveInternalDomView(componentRef.hostView.render);
-                    // We need to do this here to ensure that we create Testability and
-                    // it's ready on the window for users.
-                    registry.registerApplication(domView.boundElements[0].element, testability);
+                    registry.registerApplication(componentRef.location.nativeElement, testability);
 
                     return componentRef;
                   });
diff --git a/modules/angular2/src/core/compiler/dynamic_component_loader.ts b/modules/angular2/src/core/compiler/dynamic_component_loader.ts
@@ -36,7 +36,7 @@ export class DynamicComponentLoader {
         .then(hostProtoViewRef => {
           var hostViewRef =
               this._viewManager.createRootHostView(hostProtoViewRef, overrideSelector, injector);
-          var newLocation = new ElementRef(hostViewRef, 0);
+          var newLocation = this._viewManager.getHostElement(hostViewRef);
           var component = this._viewManager.getComponent(newLocation);
 
           var dispose = () => { this._viewManager.destroyRootHostView(hostViewRef); };
@@ -68,7 +68,7 @@ export class DynamicComponentLoader {
           var viewContainer = this._viewManager.getViewContainer(location);
           var hostViewRef =
               viewContainer.create(hostProtoViewRef, viewContainer.length, null, injector);
-          var newLocation = new ElementRef(hostViewRef, 0);
+          var newLocation = this._viewManager.getHostElement(hostViewRef);
           var component = this._viewManager.getComponent(newLocation);
 
           var dispose = () => {
diff --git a/modules/angular2/src/core/compiler/element_ref.ts b/modules/angular2/src/core/compiler/element_ref.ts
@@ -1,41 +1,22 @@
-import {DOM} from 'angular2/src/dom/dom_adapter';
-import {normalizeBlank, BaseException} from 'angular2/src/facade/lang';
+import {BaseException} from 'angular2/src/facade/lang';
 import {ViewRef} from './view_ref';
-import {resolveInternalDomView} from 'angular2/src/render/dom/view/view';
-import {RenderViewRef, RenderElementRef} from 'angular2/src/render/api';
+import {RenderViewRef, RenderElementRef, Renderer} from 'angular2/src/render/api';
 
 /**
  * @exportedAs angular2/view
  */
 export class ElementRef implements RenderElementRef {
-  constructor(public parentView: ViewRef, public boundElementIndex: number) {}
+  constructor(public parentView: ViewRef, public boundElementIndex: number,
+              private _renderer: Renderer) {}
 
-  get renderView() { return this.parentView.render; }
+  get renderView(): RenderViewRef { return this.parentView.render; }
   // TODO(tbosch): remove this once Typescript supports declaring interfaces
   // that contain getters
-  set renderView(value: any) { throw new BaseException('Abstract setter'); }
+  set renderView(viewRef: RenderViewRef) { throw new BaseException('Abstract setter'); }
 
   /**
-   * Exposes the underlying DOM element.
-   * (DEPRECATED way of accessing the DOM, replacement coming)
+   * Exposes the underlying native element.
+   * Attention: This won't work in a webworker scenario!
    */
-  // TODO(tbosch): Here we expose the real DOM element.
-  // We need a more general way to read/write to the DOM element
-  // via a proper abstraction in the render layer
-  get domElement() {
-    return resolveInternalDomView(this.parentView.render)
-        .boundElements[this.boundElementIndex]
-        .element;
-  }
-
-  /**
-   * Gets an attribute from the underlying DOM element.
-   * (DEPRECATED way of accessing the DOM, replacement coming)
-   */
-  // TODO(tbosch): Here we expose the real DOM element.
-  // We need a more general way to read/write to the DOM element
-  // via a proper abstraction in the render layer
-  getAttribute(name: string): string {
-    return normalizeBlank(DOM.getAttribute(this.domElement, name));
-  }
+  get nativeElement(): any { return this._renderer.getNativeElementSync(this); }
 }
diff --git a/modules/angular2/src/core/compiler/view.ts b/modules/angular2/src/core/compiler/view.ts
@@ -65,7 +65,7 @@ export class AppView implements ChangeDispatcher, EventDispatcher {
     this.elementRefs = ListWrapper.createFixedSize(this.proto.elementBinders.length);
     this.ref = new ViewRef(this);
     for (var i = 0; i < this.elementRefs.length; i++) {
-      this.elementRefs[i] = new ElementRef(this.ref, i);
+      this.elementRefs[i] = new ElementRef(this.ref, i, renderer);
     }
     this.locals = new Locals(null, MapWrapper.clone(protoLocals));  // TODO optimize this
   }
diff --git a/modules/angular2/src/core/compiler/view_manager.ts b/modules/angular2/src/core/compiler/view_manager.ts
@@ -30,6 +30,10 @@ export class AppViewManager {
     return hostView.elementInjectors[location.boundElementIndex].getViewContainerRef();
   }
 
+  getHostElement(hostViewRef: ViewRef): ElementRef {
+    return internalView(hostViewRef).elementRefs[0];
+  }
+
   /**
    * Returns an ElementRef for the element with the given variable name
    * in the component view of the component at the provided ElementRef.
diff --git a/modules/angular2/src/core/compiler/view_manager_utils.ts b/modules/angular2/src/core/compiler/view_manager_utils.ts
@@ -165,7 +165,7 @@ export class AppViewManagerUtils {
     if (isPresent(elementInjector.getDirectiveVariableBindings())) {
       MapWrapper.forEach(elementInjector.getDirectiveVariableBindings(), (directiveIndex, name) => {
         if (isBlank(directiveIndex)) {
-          view.locals.set(name, elementInjector.getElementRef().domElement);
+          view.locals.set(name, elementInjector.getElementRef().nativeElement);
         } else {
           view.locals.set(name, elementInjector.getDirectiveAtIndex(directiveIndex));
         }
diff --git a/modules/angular2/src/debug/debug_element.ts b/modules/angular2/src/debug/debug_element.ts
@@ -8,8 +8,6 @@ import {AppView} from 'angular2/src/core/compiler/view';
 import {internalView} from 'angular2/src/core/compiler/view_ref';
 import {ElementRef} from 'angular2/src/core/compiler/element_ref';
 
-import {resolveInternalDomView} from 'angular2/src/render/dom/view/view';
-
 /**
  * @exportedAs angular2/test
  *
@@ -35,13 +33,9 @@ export class DebugElement {
     return this._elementInjector.getComponent();
   }
 
-  get domElement(): any {
-    return resolveInternalDomView(this._parentView.render)
-        .boundElements[this._boundElementIndex]
-        .element;
-  }
+  get nativeElement(): any { return this.elementRef.nativeElement; }
 
-  get elementRef(): ElementRef { return this._elementInjector.getElementRef(); }
+  get elementRef(): ElementRef { return this._parentView.elementRefs[this._boundElementIndex]; }
 
   getDirectiveInstance(directiveIndex: number): any {
     return this._elementInjector.getDirectiveAtIndex(directiveIndex);
@@ -192,7 +186,7 @@ export class By {
   static all(): Function { return (debugElement) => true; }
 
   static css(selector: string): Function {
-    return (debugElement) => { return DOM.elementMatches(debugElement.domElement, selector); };
+    return (debugElement) => { return DOM.elementMatches(debugElement.nativeElement, selector); };
   }
   static directive(type: Type): Function {
     return (debugElement) => { return debugElement.hasDirective(type); };
diff --git a/modules/angular2/src/debug/debug_element_view_listener.ts b/modules/angular2/src/debug/debug_element_view_listener.ts
@@ -10,7 +10,7 @@ import {Injectable, bind, Binding} from 'angular2/di';
 import {AppViewListener} from 'angular2/src/core/compiler/view_listener';
 import {AppView} from 'angular2/src/core/compiler/view';
 import {DOM} from 'angular2/src/dom/dom_adapter';
-import {resolveInternalDomView} from 'angular2/src/render/dom/view/view';
+import {Renderer} from 'angular2/src/render/api';
 import {DebugElement} from './debug_element';
 
 const NG_ID_PROPERTY = 'ngid';
@@ -41,7 +41,7 @@ function _getElementId(element): List<number> {
   }
 }
 
-export function inspectDomElement(element): DebugElement {
+export function inspectNativeElement(element): DebugElement {
   var elId = _getElementId(element);
   if (isPresent(elId)) {
     var view = _allViewsById.get(elId[0]);
@@ -54,15 +54,16 @@ export function inspectDomElement(element): DebugElement {
 
 @Injectable()
 export class DebugElementViewListener implements AppViewListener {
-  constructor() { DOM.setGlobalVar(INSPECT_GLOBAL_NAME, inspectDomElement); }
+  constructor(private _renderer: Renderer) {
+    DOM.setGlobalVar(INSPECT_GLOBAL_NAME, inspectNativeElement);
+  }
 
   viewCreated(view: AppView) {
     var viewId = _nextId++;
     _allViewsById.set(viewId, view);
     _allIdsByView.set(view, viewId);
-    var renderView = resolveInternalDomView(view.render);
-    for (var i = 0; i < renderView.boundElements.length; i++) {
-      _setElementId(renderView.boundElements[i].element, [viewId, i]);
+    for (var i = 0; i < view.elementRefs.length; i++) {
+      _setElementId(this._renderer.getNativeElementSync(view.elementRefs[i]), [viewId, i]);
     }
   }
 
diff --git a/modules/angular2/src/directives/class.ts b/modules/angular2/src/directives/class.ts
@@ -2,29 +2,23 @@ import {Directive, onCheck} from 'angular2/annotations';
 import {ElementRef} from 'angular2/core';
 import {PipeRegistry} from 'angular2/src/change_detection/pipes/pipe_registry';
 import {isPresent} from 'angular2/src/facade/lang';
-import {DOM} from 'angular2/src/dom/dom_adapter';
+import {Renderer} from 'angular2/src/render/api';
 
 @Directive({selector: '[class]', lifecycle: [onCheck], properties: ['rawClass: class']})
 export class CSSClass {
-  _domEl;
   _pipe;
   _rawClass;
 
-  constructor(private _pipeRegistry: PipeRegistry, ngEl: ElementRef) {
-    this._domEl = ngEl.domElement;
-  }
+  constructor(private _pipeRegistry: PipeRegistry, private _ngEl: ElementRef,
+              private _renderer: Renderer) {}
 
   set rawClass(v) {
     this._rawClass = v;
     this._pipe = this._pipeRegistry.get('keyValDiff', this._rawClass);
   }
 
   _toggleClass(className, enabled): void {
-    if (enabled) {
-      DOM.addClass(this._domEl, className);
-    } else {
-      DOM.removeClass(this._domEl, className);
-    }
+    this._renderer.setElementClass(this._ngEl, className, enabled);
   }
 
   onCheck() {
@@ -38,7 +32,7 @@ export class CSSClass {
       diff.forEachChangedItem((record) => { this._toggleClass(record.key, record.currentValue); });
       diff.forEachRemovedItem((record) => {
         if (record.previousValue) {
-          DOM.removeClass(this._domEl, record.key);
+          this._toggleClass(record.key, false);
         }
       });
     }
diff --git a/modules/angular2/src/render/api.ts b/modules/angular2/src/render/api.ts
@@ -370,6 +370,12 @@ export class Renderer {
    */
   dehydrateView(viewRef: RenderViewRef) {}
 
+  /**
+   * Returns the native element at the given location.
+   * Attention: In a WebWorker scenario, this should always return null!
+   */
+  getNativeElementSync(location: RenderElementRef): any { return null; }
+
   /**
    * Sets a property on an element.
    */
diff --git a/modules/angular2/src/render/dom/dom_renderer.ts b/modules/angular2/src/render/dom/dom_renderer.ts
@@ -53,6 +53,12 @@ export class DomRenderer extends Renderer {
     // noop for now
   }
 
+  getNativeElementSync(location: RenderElementRef) {
+    return resolveInternalDomView(location.renderView)
+        .boundElements[location.boundElementIndex]
+        .element;
+  }
+
   attachComponentView(location: RenderElementRef, componentViewRef: RenderViewRef) {
     var hostView = resolveInternalDomView(location.renderView);
     var componentView = resolveInternalDomView(componentViewRef);
diff --git a/modules/angular2/src/router/router_link.ts b/modules/angular2/src/router/router_link.ts
@@ -4,10 +4,10 @@ import {ElementRef} from 'angular2/core';
 import {StringMap, StringMapWrapper} from 'angular2/src/facade/collection';
 
 import {isPresent} from 'angular2/src/facade/lang';
-import {DOM} from 'angular2/src/dom/dom_adapter';
 
 import {Router} from './router';
 import {Location} from './location';
+import {Renderer} from 'angular2/src/render/api';
 
 /**
  * The RouterLink directive lets you link to specific parts of your app.
@@ -37,7 +37,6 @@ import {Location} from './location';
   host: {'(^click)': 'onClick()'}
 })
 export class RouterLink {
-  private _domEl;
   private _route: string;
   private _params: StringMap<string, string>;
 
@@ -46,8 +45,8 @@ export class RouterLink {
   // the url passed to the router navigation.
   _navigationHref: string;
 
-  constructor(elementRef: ElementRef, private _router: Router, private _location: Location) {
-    this._domEl = elementRef.domElement;
+  constructor(private _elementRef: ElementRef, private _router: Router, private _location: Location,
+              private _renderer: Renderer) {
     this._params = StringMapWrapper.create();
   }
 
@@ -66,7 +65,7 @@ export class RouterLink {
       this._visibleHref = this._location.normalizeAbsolutely(this._navigationHref);
       // Keeping the link on the element to support contextual menu `copy link`
       // and other in-browser affordances.
-      DOM.setAttribute(this._domEl, 'href', this._visibleHref);
+      this._renderer.setElementAttribute(this._elementRef, 'href', this._visibleHref);
     }
   }
 }
diff --git a/modules/angular2/src/services/ruler.ts b/modules/angular2/src/services/ruler.ts
@@ -24,7 +24,7 @@ export class Ruler {
   constructor(domAdapter: DomAdapter) { this.domAdapter = domAdapter; }
 
   measure(el: ElementRef): Promise<Rectangle> {
-    var clntRect = <any>this.domAdapter.getBoundingClientRect(el.domElement);
+    var clntRect = <any>this.domAdapter.getBoundingClientRect(el.nativeElement);
 
     // even if getBoundingClientRect is synchronous we use async API in preparation for further
     // changes
diff --git a/modules/angular2/test/core/compiler/dynamic_component_loader_spec.ts b/modules/angular2/test/core/compiler/dynamic_component_loader_spec.ts
@@ -41,7 +41,7 @@ export function main() {
 
                         loader.loadIntoLocation(DynamicallyLoaded, tc.elementRef, 'loc')
                             .then(ref => {
-                              expect(tc.domElement).toHaveText("Location;DynamicallyLoaded;");
+                              expect(tc.nativeElement).toHaveText("Location;DynamicallyLoaded;");
                               async.done();
                             });
                       });
@@ -60,7 +60,7 @@ export function main() {
                         loader.loadIntoLocation(DynamicallyLoaded, tc.elementRef, 'loc')
                             .then(ref => {
                               ref.dispose();
-                              expect(tc.domElement).toHaveText("Location;");
+                              expect(tc.nativeElement).toHaveText("Location;");
                               async.done();
                             });
                       });
@@ -81,7 +81,7 @@ export function main() {
 
                            tc.detectChanges();
 
-                           var newlyInsertedElement = DOM.childNodes(tc.domElement)[1];
+                           var newlyInsertedElement = DOM.childNodes(tc.nativeElement)[1];
                            expect(newlyInsertedElement.id)
                                .toEqual("new value")
 
diff --git a/modules/angular2/test/core/compiler/query_integration_spec.ts b/modules/angular2/test/core/compiler/query_integration_spec.ts
@@ -256,8 +256,8 @@ export function main() {
 
                  view.detectChanges();
 
-                 expect(q.query.first.domElement).toHaveText("1d");
-                 expect(q.query.last.domElement).toHaveText("2d");
+                 expect(q.query.first.nativeElement).toHaveText("1d");
+                 expect(q.query.last.nativeElement).toHaveText("2d");
 
                  async.done();
                });
diff --git a/modules/angular2/test/core/compiler/view_container_ref_spec.ts b/modules/angular2/test/core/compiler/view_container_ref_spec.ts
@@ -16,14 +16,12 @@ import {
   proxy
 } from 'angular2/test_lib';
 
-import {MapWrapper} from 'angular2/src/facade/collection';
 import {IMPLEMENTS} from 'angular2/src/facade/lang';
 
 import {AppView, AppProtoView, AppViewContainer} from 'angular2/src/core/compiler/view';
-import {ProtoViewRef, ViewRef, internalView} from 'angular2/src/core/compiler/view_ref';
-import {ElementRef} from 'angular2/src/core/compiler/element_ref';
 import {ViewContainerRef} from 'angular2/src/core/compiler/view_container_ref';
 import {AppViewManager} from 'angular2/src/core/compiler/view_manager';
+import {ElementBinder} from 'angular2/src/core/compiler/element_binder';
 
 export function main() {
   // TODO(tbosch): add missing tests
@@ -33,9 +31,11 @@ export function main() {
     var view;
     var viewManager;
 
-    function wrapView(view: AppView): ViewRef { return new ViewRef(view); }
-
-    function createProtoView() { return new AppProtoView(null, null, null, null); }
+    function createProtoView() {
+      var pv = new AppProtoView(null, null, null, null);
+      pv.elementBinders = [new ElementBinder(0, null, 0, null, null)];
+      return pv;
+    }
 
     function createView() { return new AppView(null, createProtoView(), new Map()); }
 
@@ -45,7 +45,7 @@ export function main() {
       viewManager = new AppViewManagerSpy();
       view = createView();
       view.viewContainers = [null];
-      location = new ElementRef(wrapView(view), 0);
+      location = view.elementRefs[0];
     });
 
     describe('length', () => {
diff --git a/modules/angular2/test/core/compiler/view_manager_spec.ts b/modules/angular2/test/core/compiler/view_manager_spec.ts
diff --git a/modules/angular2/test/debug/debug_element_spec.ts b/modules/angular2/test/debug/debug_element_spec.ts
diff --git a/modules/angular2/test/debug/debug_element_view_listener_spec.ts b/modules/angular2/test/debug/debug_element_view_listener_spec.ts
diff --git a/modules/angular2/test/directives/non_bindable_spec.ts b/modules/angular2/test/directives/non_bindable_spec.ts
diff --git a/modules/angular2/test/services/ruler_spec.ts b/modules/angular2/test/services/ruler_spec.ts
diff --git a/modules/angular2/test/test_lib/test_component_builder_spec.ts b/modules/angular2/test/test_lib/test_component_builder_spec.ts
diff --git a/modules/examples/src/hello_world/index_common.ts b/modules/examples/src/hello_world/index_common.ts

Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,2 @@`
`1`	`1`	`export * from './src/debug/debug_element';`
`2`		`-export {inspectDomElement, ELEMENT_PROBE_CONFIG} from './src/debug/debug_element_view_listener';`
	`2`	`+export {inspectNativeElement, ELEMENT_PROBE_CONFIG} from './src/debug/debug_element_view_listener';`
Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ export class AppView implements ChangeDispatcher, EventDispatcher {`
`65`	`65`	`this.elementRefs = ListWrapper.createFixedSize(this.proto.elementBinders.length);`
`66`	`66`	`this.ref = new ViewRef(this);`
`67`	`67`	`for (var i = 0; i < this.elementRefs.length; i++) {`
`68`		`- this.elementRefs[i] = new ElementRef(this.ref, i);`
	`68`	`+ this.elementRefs[i] = new ElementRef(this.ref, i, renderer);`
`69`	`69`	`}`
`70`	`70`	`this.locals = new Locals(null, MapWrapper.clone(protoLocals)); // TODO optimize this`
`71`	`71`	`}`
Original file line number	Diff line number	Diff line change
`@@ -165,7 +165,7 @@ export class AppViewManagerUtils {`
`165`	`165`	`if (isPresent(elementInjector.getDirectiveVariableBindings())) {`
`166`	`166`	`MapWrapper.forEach(elementInjector.getDirectiveVariableBindings(), (directiveIndex, name) => {`
`167`	`167`	`if (isBlank(directiveIndex)) {`
`168`		`- view.locals.set(name, elementInjector.getElementRef().domElement);`
	`168`	`+ view.locals.set(name, elementInjector.getElementRef().nativeElement);`
`169`	`169`	`} else {`
`170`	`170`	`view.locals.set(name, elementInjector.getDirectiveAtIndex(directiveIndex));`
`171`	`171`	`}`