Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

ngSwitch error when using Angular with Polymer's platform.js loaded #8598

Closed
theefer opened this issue Aug 13, 2014 · 24 comments
Closed

ngSwitch error when using Angular with Polymer's platform.js loaded #8598

theefer opened this issue Aug 13, 2014 · 24 comments

Comments

@theefer
Copy link

theefer commented Aug 13, 2014

Context: As part of a large AngularJS application, we're trying to bring in a small Polymer Web Component. platform.js is loaded first in the page, and Angular is manually bootstrapped on a wrapped document node.

It all works fine in latest Chromium (likely because the platform.js polyfills don't do much as most/all? APIs exist natively), but fails with an error in FF 31. I've isolated the error to nested ngSwitch, which result in a slightly weird minimal failing reduced case that you can see at http://jsbin.com/roxif/6/edit

The offending code is the following:

<div ng:switch="3">
  <div ng:switch-when="3">
    <div>
      <div ng:switch="3">
        <div ng:switch-when="3">3</div>
      </div>
    </div>
  </div>
</div>

Note that the extra wrapping div in between the nested ngSwitches is required for the error to occur. Without that extra element, the error disappears.

The following error gets thrown:

Error: node is undefined
compositeLinkFn@https://rawgit.com/angular/bower-angular/v1.2.16/angular.js:5989:13
nodeLinkFn@https://rawgit.com/angular/bower-angular/v1.2.16/angular.js:6573:9
compositeLinkFn@https://rawgit.com/angular/bower-angular/v1.2.16/angular.js:5983:1
nodeLinkFn@https://rawgit.com/angular/bower-angular/v1.2.16/angular.js:6573:9
compositeLinkFn@https://rawgit.com/angular/bower-angular/v1.2.16/angular.js:5986:15
publicLinkFn@https://rawgit.com/angular/bower-angular/v1.2.16/angular.js:5891:30
ngViewFillContentFactory/<.link@https://rawgit.com/angular/bower-angular-route/v1.2.16/angular-route.js:921:7
nodeLinkFn@https://rawgit.com/angular/bower-angular/v1.2.16/angular.js:6580:1
compositeLinkFn@https://rawgit.com/angular/bower-angular/v1.2.16/angular.js:5986:15
publicLinkFn@https://rawgit.com/angular/bower-angular/v1.2.16/angular.js:5891:30
boundTranscludeFn@https://rawgit.com/angular/bower-angular/v1.2.16/angular.js:6005:13
controllersBoundTransclude@https://rawgit.com/angular/bower-angular/v1.2.16/angular.js:6600:11
update@https://rawgit.com/angular/bower-angular-route/v1.2.16/angular-route.js:871:17
$RootScopeProvider/this.$get</Scope.prototype.$broadcast@https://rawgit.com/angular/bower-angular/v1.2.16/angular.js:12691:15
updateRoute/<@https://rawgit.com/angular/bower-angular-route/v1.2.16/angular-route.js:552:15
etc...

It looks like the error maps to the last line of the following function (node.childNodes where node is undefined):

      function compositeLinkFn(scope, nodeList, $rootElement, boundTranscludeFn) {
        var nodeLinkFn, childLinkFn, node, $node, childScope, childTranscludeFn, i, ii, n;

        // copy nodeList so that linking doesn't break due to live list updates.
        var nodeListLength = nodeList.length,
            stableNodeList = new Array(nodeListLength);
        for (i = 0; i < nodeListLength; i++) {
          stableNodeList[i] = nodeList[i];
        }

        for(i = 0, n = 0, ii = linkFns.length; i < ii; n++) {
          node = stableNodeList[n];
          nodeLinkFn = linkFns[i++];
          childLinkFn = linkFns[i++];
          $node = jqLite(node);

          if (nodeLinkFn) {
            if (nodeLinkFn.scope) {
              childScope = scope.$new();
              $node.data('$scope', childScope);
            } else {
              childScope = scope;
            }
            childTranscludeFn = nodeLinkFn.transclude;
            if (childTranscludeFn || (!boundTranscludeFn && transcludeFn)) {
              nodeLinkFn(childLinkFn, childScope, node, $rootElement,
                createBoundTranscludeFn(scope, childTranscludeFn || transcludeFn)
              );
            } else {
              nodeLinkFn(childLinkFn, childScope, node, $rootElement, boundTranscludeFn);
            }
          } else if (childLinkFn) {
            childLinkFn(scope, node.childNodes, undefined, boundTranscludeFn);
          }
        }

Note that this fails even with the latest stable Angular version (1.2.22) as per this other example and also with the latest unstable Angular version (1.3.0-beta.18) as per this example.

It seems likely that this is a broken edge case of the wrapping platform.js does to polyfill Shadow DOM in FF?

Any hints as to where this error comes from and how to solve it?

I will cross-file this issue in the Polymer bug tracker as they may also be interested to know about this.

Thanks in advance.

@theefer
Copy link
Author

theefer commented Aug 13, 2014

/cc @addyosmani :-)

@btford
Copy link
Contributor

btford commented Aug 18, 2014

This is a bug with platform.js then, correct?

@btford btford added this to the Purgatory milestone Aug 18, 2014
@theefer
Copy link
Author

theefer commented Aug 19, 2014

I suspect it may be, although it was only exhibited when used in conjunction with AngularJS. I'm going to prod the related platform.js issue, thanks.

@jmesserly
Copy link

what seems to be happening is in compileNodes compositeLinkFn callback, linkFns ends up a different size from the stableNodeList. I'm not familiar with the Angular code to know what this might mean...

One constraint of the ShadowDOM polyfill is that the .childNodes list isn't live. So that's something to look for in the Angular code. However this particular function seems to want a snapshot, so the limitation might not be relevant.

@jmesserly
Copy link

another note: the node without children is <!-- ngSwitchWhen: 3 -->. tree structure at this point is <!-- ngView: --><div class="ng-scope" ng:view=""><p class="ng-scope">OK</p><div class="ng-scope" ng:switch="3"><!-- ngSwitchWhen: 3 --></div></div>

@jmesserly
Copy link

One constraint of the ShadowDOM polyfill is that the .childNodes list isn't live. So that's something to look for in the Angular code.

Ah, I think that's the issue. In compileNodes there's this comment:

// we must always refer to nodeList[i] since the nodes can be replaced underneath us.

however, under ShadowDOM polyfill that won't work -- nodeList[i] will always return the same value.

now that I look at the example again, I noticed it uses Angular 1. fwiw, I'm not sure Angular 1 is designed to work with platform.js in mind. I'm pretty sure this works in Angular 2, because it has been tested against platform.js. For reference: https://github.com/Polymer/ShadowDOM#known-limitations.

Could someone from Angular comment how feasible it would be to refactor the code to not rely on live NodeLists? I'm guessing it would be pretty simple (instead of nodeList[i] use node.childNodes[i]), but without a deeper dive I'm not sure.

@theefer
Copy link
Author

theefer commented Aug 20, 2014

Thanks a lot @jmesserly for looking into this!

I'm pretty sure this works in Angular 2, because it has been tested against platform.js.

Is there an Angular 2 preview branch somewhere? Unfortunately it is still failing in Angular 1.3-beta.18 (http://jsbin.com/baviy/1/edit).

@jmesserly
Copy link

Ah, that's a good question. I'm not sure, just saw the announcement http://blog.angularjs.org/2014/03/angular-20.html and it has a link to the design docs, which are all public. There's also http://ng-learn.org/2014/03/AngularJS-2-Status-Preview/ and https://github.com/angular/angular.dart but I don't know if there's an AngularJS 2 repo up somewhere...

@jmesserly
Copy link

... and it's still be good to get this fixed in 1.0 if it's possible ... I might try a pull request, but was just wondering if someone from AngularJS project could tell me if the proposed changes sounded crazy before I started. :)

@caitp
Copy link
Contributor

caitp commented Aug 20, 2014

but I don't know if there's an AngularJS 2 repo up somewhere..

There are lots of angular 2.0 repos up, and they're all public.

https://github.com/angular/di.js
https://github.com/angular/prophecy
https://github.com/angular/router
https://github.com/angular/templating
https://github.com/angular/http
https://github.com/angular/watchtower.js
https://github.com/angular/projects

I'm not sure if zone.js counts, but lets say it does.


The one that for this question you probably care about is "templating"

@jmesserly
Copy link

Awesome, thank you!

@caitp
Copy link
Contributor

caitp commented Aug 20, 2014

So the interesting thing in this case is that in Chrome, we never add the new childLinkFn to the list of linkFns to execute (thus, the path where we'd have an undefined node never gets run into), but in Firefox (or presumably any case where the polyfills are doing something meaningful), the childLinkFn does get added, and we try to run the composite link function on each child of (in this case the comment node, so the child node list length is always 0).

Adding an extra condition like if (childLinkFn && node) instead of just if (childLinkFn) would probably be enough to fix it, but I'm happy to poke it and see if I can find a better fix.

The harder problem is that it's somewhat difficult to test this :*(

@IgorMinar
Copy link
Contributor

that means that platform.js does something that makes us take a different path, which should be corrected in the polyfill. we should find out what it is a let the chrome guys know.

@jmesserly
Copy link

Hi Igor, depending on what the reason is is, it might be impossible to fix in platform.js :(. It is not possible to create a live NodeList, for example. If the code relies on that we can't fix it from the platform.js side, as much as I wish it was possible.

@IgorMinar
Copy link
Contributor

I doubt that the live NodeList is the culprit here. It's something else.
On Aug 21, 2014 7:19 PM, "John Messerly" [email protected] wrote:

Hi Igor, depending on what the reason is is, it might be impossible to fix
in platform.js :(. It is not possible to create a live NodeList, for
example. If the code relies on that we can't fix it from the platform.js
side, as much as I wish it was possible.


Reply to this email directly or view it on GitHub
#8598 (comment).

@theefer
Copy link
Author

theefer commented Sep 2, 2014

It sounds like a fix on the Polymer side is not an option, and they have closed the associated issue (https://github.com/Polymer/platform/issues/78). While I understand it's rather inelegant to have to work around the limitations of a polyfill in an unrelated library, it seems to be the only way?

In that case, is everyone happy with the idea of patching Angular to allow using it in conjunction with Polymer web components (maybe taking clues from @caitp's suggestion)? Currently, it's not possible, even though it's meant to be a selling point of both Angular and Polymer.

@theefer
Copy link
Author

theefer commented Jan 1, 2015

This issue still seems to be occurring even after I bumped the jsbin to use the latest versions of angular (1.3.8) and platform webcomponents.js (0.5.2).

I'd be keen to help get this fixed if someone can point me in the right direction, as we'd like to start using web components within an Angular app, and this is currently a blocker.

I could try and pursue the suggestions in the thread above, but @IgorMinar you suggested the problem is not caused by the live NodeList? If not, do you have any ideas and we could address this problem?

/cc @robdodson

@addyosmani
Copy link

Issue 78 appears to have been moved as part of our repo clean-up after consolidating platform.js into https://github.com/webcomponents/webcomponentsjs.

After digging into the issue briefly mentioned, I can't see anything other than the NodeList (nodeList[i]) limitations under ShadowDOM polyfill being at fault here. You could try digging into that polyfill (see https://github.com/webcomponents/webcomponentsjs/tree/master/src/ShadowDOM) to try monkey-patching it, but I would check with @jmesserly if that's even possible.

@theefer
Copy link
Author

theefer commented Jan 1, 2015

OK, I might give it a go in the coming days, thanks.

@jmesserly
Copy link

as far as I know, it's not possible to patch the indexer, unless you can depend on ES6 Proxies. Once proxies are everywhere we could return a proxy. Though I'm unsure the performance implications of going that route for NodeList, because it is often used in performance critical parts of the code.

One other approach that might work with currently available EcmaScript 5: instead of building childNodes/children on demand, cache them and mutate in every appendChild/insertBefore/removeChild operation. This is possible because SD polyfill traps these calls. Problem with that is it might be expensive, especially for inserts in the middle of the list. Then again, computing those lists on-demand is also expensive in its own way, so it might be a wash.

If you're up for the experiment, probably the place to start is at https://github.com/webcomponents/webcomponentsjs/blob/master/src/ShadowDOM/wrappers/Node.js#L660 ... have that return this.childNodes_, which is a cached list. Then change insertBefore/removeChild to update the list. Probably could be done about here: https://github.com/webcomponents/webcomponentsjs/blob/master/src/ShadowDOM/wrappers/Node.js#L426

edit: and it would likely need to initialize the list on demand too. Perhaps in the wrapper constructor.

@bkardell
Copy link

I'm honestly not sure if this is related but I have a custom element - just the custom element with non-polymer polyfill, so it actually mutates the DOM, there's no shadow... This actually seems to work fine (if you make your element carefully so that it is kind of self-aware that it can be in an uplifted state or not), you can see an example in this codepen http://codepen.io/bkardell/pen/NPMqeY - however, for some reason if we use angular-ui-router I see this same error in non-chrome browsers (so, the polyfilled ones) http://plnkr.co/edit/icl2EgMHwnXpu6phbX2a?p=preview there (click to show the list of items). I realize this is -likely- a different bug, but I'm wondering if anyone understands the internals enough to understand why it appears that in some cases that stableNodeList/linkNodes gets hosed so I can figure out which angle/project to chase this from.

@ghost
Copy link

ghost commented May 14, 2015

I'm experiencing this issue on Safari when using ngSwitch in a template. Replacing ngSwitch with ngIf is my current workaround.

undefined is not an object (evaluating 'linkNode.childNodes')
nodeLinkFn@http://localhost:4567/components/angular/angular.js:8029:58
compositeLinkFn@http://localhost:4567/components/angular/angular.js:7435:23

@lgalfaso
Copy link
Contributor

lgalfaso commented Mar 6, 2016

There is nothing Angular is doing out of the spec. This is an issue in platform.js and should be fixed there.

@morewry
Copy link

morewry commented Oct 31, 2018

Potentially of interest webcomponents/shadydom#286

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants