Nativescript-angular: @ViewChild fails on built-in {N} components given their class name

Created on 30 Oct 2017  ยท  6Comments  ยท  Source: NativeScript/nativescript-angular

Overview

Angular is unable to match queries against class names for built-in {N} components. The following basic view query will fail and return undefined.

@Component({
  selector: 'comp-1',
  template: `<GridLayout></GridLayout>`
})
class CompOne implements AfterViewInit {
  @ViewChild(GridLayout) ref: GridLayout;
  ngAfterViewInit() {
    console.log("child ref is -> " + this.ref);
  }
}

Reproduction
https://play.nativescript.org/?id=ezJW7O_-Yw0QbA-pQC280&template=play-ng

Why it fails
The best way I've had it explained was by @vakrilov in Slack: The built-in {N} components are similar to the native elements in a Browser (HTMLButtonElement, HTMLInputElement, etc.) You would not query for @ViewChild(HTMLInputElement) in the Browser, so why would you query for @ViewChild(Image) or @ViewChild(Label) in {N}+Angular?

Problems
The browser element analogy makes sense as a way to explain the limitation, but it breaks down under scrutiny from a developer experience point of view. In the browser case all of the built-in elements are named consistently HTML[name]Element, so you know immediately by name if an element can be queried for using its class name. In the {N} case the built-ins are called "components" in the docs, and have no hint in their names (Image, Label, etc.) to let you know they're built-in so you have to think about it each time you write a view query.

The point I'm making here is _subtle_, and I understand that, but I think it's worth discussing as it has the potential to address a common stumbling block and/or point of confusion for developers getting started with {N}+Angular.

Solutions
I see at least a few ways you could try to address this:

  1. Use developer education by way of tutorials and documentation updates to make this limitation clear and understandable to users. I suspect this is a challenging way to go about things because it's hard to get people to read things before they go ask someone else for help.

  2. Update built-in names with common convention to make it more obvious that they're native elements. I don't seriously think anyone would attempt this, but it's an option.

  3. Throw an error when a developer writes a query for a built-in element. This would give developers timely feedback and keep them moving without lifting the query restriction. I don't know how realistic this is because the ViewQuery stuff is pretty buried in angular's guts and hooking it might not be straightforward.

  4. Declare built-ins as components to Angular. I believe that {N} has all the information it needs to do this. It already has a hook for declaring components registerElement which could potentially be used to decorate the view classes with @Directive and add them to an Angular module that is imported. I have verified that built-ins are queried if they're decorated and declared as components. You could even go further to map all their {N} declared properties as @Inputs.

  5. Keep this issue closed, and maybe it's a good reference for when people stumble.

docs question

Most helpful comment

Hey @justindujardin - Thanks for the detailed description!
I'm reopening this for discussion. Here are my thoughts on the points mentioned:

  1. Improving the docs and explain things better is definitely needed here to avoid confusion.
  2. Actually, we only need to update how the elements are registered(registerElement) in the renderer. However, in the current state, I think such renaming will cause more confusion than solve problem.
  3. Not sure if this can be done, but if possible - sounds like a very good option.
  4. As discussed - NativeScript Views are equivalent to dom-elements in web-angular. The angular guides are quite clear about accessing the dom directly - avoid that as much as possible. Making them easier to query will encourage that. The other big concern that I have to this approach is that means you will heave much more angular components in your app (as all the layouts and elements will also be components). This means more objects, more memory and slower change-detection passes.
  5. I think this issue is useful reference point already. Thanks for that!

_Recap:_ My vote is to implement 1.(improve the docs) and research 3.(try to get better errors messages if anyone tries to select a View class). If you have other thoughts and suggestion let's discuss them.

All 6 comments

I can confirm, happens also here. Stacktrace for reference:

TypeError: undefined is not an object (evaluating 'this._selectors.query') matchSelectors โ€” style-scope.js:491 updateMatch โ€” style-scope.js:258:84 onLoaded โ€” style-scope.js:249 onLoaded โ€” view-base.js:183 _addViewCore โ€” view-base.js:344 _addViewCore โ€” view.js:23 _addView โ€” view-base.js:330 set โ€” content-view.js:23 addToVisualTree โ€” view-util.js:76 insertChild โ€” view-util.js:41 appendChild โ€” renderer.js:73 appendChild โ€” services.ts:943 createElement โ€” element.ts:196 createViewNodes โ€” view.ts:307 callViewAction โ€” view.ts:782 execComponentViewsAction โ€” view.ts:700 createViewNodes โ€” view.ts:369 createRootView โ€” view.ts:225 callWithDebugContext โ€” services.ts:815 create โ€” refs.ts:130 bootstrap โ€” application_ref.ts:661 forEach _moduleDoBootstrap โ€” application_ref.ts:411 file:///app/tns_modules/ onInvoke โ€” ng_zone.ts:296 run โ€” zone-nativescript.js:125 (anonymous function) โ€” zone-nativescript.js:760 onInvokeTask โ€” ng_zone.ts:288 runTask โ€” zone-nativescript.js:165 drainMicroTaskQueue โ€” zone-nativescript.js:593 promiseReactionJob require cubicBezier โ€” enums.js:178 parseCubicBezierCurve โ€” converters.js:34 animationTimingFunctionConverter โ€” converters.js:21 keyframesArrayFromCSS โ€” css-animation-parser.js:73:87 _applyKeyframesOnSelectors โ€” style-scope.js:515 _createSelectors โ€” style-scope.js:485 ensureSelectors โ€” style-scope.js:470 matchSelectors โ€” style-scope.js:490 updateMatch โ€” style-scope.js:258:84 onLoaded โ€” style-scope.js:249 onLoaded โ€” view-base.js:183 onLoaded โ€” page.js:262 _addViewCore โ€” view-base.js:344 _addViewCore โ€” view.js:23 _addView โ€” view-base.js:330 viewWillAppear โ€” page.js:134 UIApplicationMain start โ€” application.js:213 bootstrapApp โ€” platform-common.js:84 bootstrapModule โ€” platform-common.js:72 anonymous โ€” main.ts:33 evaluate moduleEvaluation [native code] promiseReactionJob

@justindujardin , @noomieware accessing via Class name is not supported for the native elements in the NativeScript modules.
Instead, use id and ElementRef.

For example:

import { Component, ViewChild, ElementRef } from "@angular/core";
import { GridLayout } from "ui/layouts/grid-layout";

@Component({
    selector: "ns-items",
    moduleId: module.id,
    templateUrl: "./items.component.html",
})
export class ItemsComponent {

    @ViewChild("grid") ref : ElementRef;

    ngAfterViewInit() {
        console.log(this.ref);
        console.log(this.ref.nativeElement); // GridLayout

        let myGrid = <GridLayout>this.ref.nativeElement;
    }
}

And in the HTML file assign the Angular id

<GridLayout #grid >
</GridLayout>

Oh, then I have to open a separate issue, because my stack is not caused by the exact same root.

@NickIliev thanks for the prompt response. Sorry for the less than adequate original post. I updated it to be more clear about my intent. Can you open this back up for conversation?

Hey @justindujardin - Thanks for the detailed description!
I'm reopening this for discussion. Here are my thoughts on the points mentioned:

  1. Improving the docs and explain things better is definitely needed here to avoid confusion.
  2. Actually, we only need to update how the elements are registered(registerElement) in the renderer. However, in the current state, I think such renaming will cause more confusion than solve problem.
  3. Not sure if this can be done, but if possible - sounds like a very good option.
  4. As discussed - NativeScript Views are equivalent to dom-elements in web-angular. The angular guides are quite clear about accessing the dom directly - avoid that as much as possible. Making them easier to query will encourage that. The other big concern that I have to this approach is that means you will heave much more angular components in your app (as all the layouts and elements will also be components). This means more objects, more memory and slower change-detection passes.
  5. I think this issue is useful reference point already. Thanks for that!

_Recap:_ My vote is to implement 1.(improve the docs) and research 3.(try to get better errors messages if anyone tries to select a View class). If you have other thoughts and suggestion let's discuss them.

@vakrilov You bet, thanks for the excellent software!

Your measured approach sounds good to me. I agree that #3 could be a fine solution. Here are a few ideas:

@ViewChild throws during view init
Angular has a set of reflection APIs that can be used to pull decorator data off of classes and properties. We could inject an array of string blacklisted elements and search through the view properties to find bad queries. Adding an assertion to view init would probably be pretty straightforward to implement but may require a PR to angular/angular if {N} doesn't already have hooks for customizing lifecycle callbacks. Considering performance, you would probably want to only assert in debug mode.

Ahead of Time compiler errors
Another approach might be to look at the AoT compiler because it already does a bunch of reflection. The downside to using AoT compiler is that you won't get the feedback during regular development, which is what you would want to encourage a quick find/fix. This may also require a PR to angular/angular.

Codelyzer plugin
Codelyzer is great for catching errors in angular apps and has an extensible setup. Introducing Codelyzer might be a pain and require updating all template/seed projects in order to get comprehensive coverage and protections.

Metadata Resolver
Angular has a number of assertions that it does already when compiling components and directives. Perhaps a new assertion could be added here. Inject a token for blacklisted view query classes like the ViewChild example above.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

bhavincb picture bhavincb  ยท  3Comments

tsonevn picture tsonevn  ยท  3Comments

EricRobertBrewer picture EricRobertBrewer  ยท  3Comments

VladimirAmiorkov picture VladimirAmiorkov  ยท  3Comments

triniwiz picture triniwiz  ยท  3Comments