googlearchive / TemplateBinding

TemplateBinding Prolyfill
290 stars 61 forks source link

Attempt to call null.querySelector() in <template>.ref after upgrade to Polymer 0.1.2 #152

Closed jyasskin closed 10 years ago

jyasskin commented 10 years ago

To reproduce, clone git@github.com:cplusplus/fundamentals-ts.git (at 8bb73e5b0), run bower update, run a web server in that directory, and try to load /fundamentals-ts.html. Something in https://github.com/cplusplus/fundamentals-ts/blob/master/elements/toc.html causes the following stack trace. The second .ref gets to what I think is https://github.com/cplusplus/fundamentals-ts/blob/8bb73e5b05bf7f0770a149fb1b27cb6258bbefa0/elements/toc.html#L23, hits a #document-fragment with no templateCreator_ or getElementById(), and dies.

Uncaught TypeError: Cannot call method 'querySelector' of null TemplateBinding.js:499 ref TemplateBinding.js:499 ref TemplateBinding.js:509 mixin.createInstance TemplateBinding.js:419 (anonymous function) TemplateBinding.js:986 TemplateIterator.handleSplices TemplateBinding.js:970 TemplateIterator.valueChanged TemplateBinding.js:866 TemplateIterator.updateIteratedValue TemplateBinding.js:849 TemplateIterator.updateDependencies TemplateBinding.js:829 mixin.processBindingDirectives TemplateBinding.js:414 processBindings TemplateBinding.js:658 cloneAndBindingInstance TemplateBinding.js:749 cloneAndBindingInstance TemplateBinding.js:736 mixin.createInstance TemplateBinding.js:440 (anonymous function) TemplateBinding.js:986 TemplateIterator.handleSplices TemplateBinding.js:970 TemplateIterator.valueChanged TemplateBinding.js:866 TemplateIterator.updateIteratedValue TemplateBinding.js:849 TemplateIterator.updateDependencies TemplateBinding.js:829 mixin.processBindingDirectives TemplateBinding.js:414 processBindings TemplateBinding.js:658 cloneAndBindingInstance TemplateBinding.js:749 cloneAndBindingInstance TemplateBinding.js:736 mixin.createInstance TemplateBinding.js:440 (anonymous function) TemplateBinding.js:986 TemplateIterator.handleSplices TemplateBinding.js:970 TemplateIterator.valueChanged TemplateBinding.js:866 TemplateIterator.updateIteratedValue TemplateBinding.js:849 TemplateIterator.updateDependencies TemplateBinding.js:829 mixin.processBindingDirectives TemplateBinding.js:414 processBindings TemplateBinding.js:658 cloneAndBindingInstance TemplateBinding.js:749 mixin.createInstance TemplateBinding.js:440 (anonymous function) TemplateBinding.js:986 TemplateIterator.handleSplices TemplateBinding.js:970 TemplateIterator.valueChanged TemplateBinding.js:866 TemplateIterator.updateIteratedValue TemplateBinding.js:849 TemplateIterator.updateDependencies TemplateBinding.js:829 mixin.processBindingDirectives TemplateBinding.js:414 processBindings TemplateBinding.js:658 cloneAndBindingInstance TemplateBinding.js:749 cloneAndBindingInstance TemplateBinding.js:736 mixin.createInstance TemplateBinding.js:440 mdv.instanceTemplate mdv.js:33 base.shadowFromTemplate base.js:127 base.parseDeclaration base.js:105 base.parseDeclarations base.js:95 base.prepareElement base.js:42 base.createdCallback base.js:24 prototype.registerPrototype prototype.js:188 prototype.register prototype.js:26 extend._register polymer-element.js:61 extend.registerWhenReady polymer-element.js:56 notifyPrototype polymer-element.js:122 element polymer-element.js:21 (anonymous function) toc.js:2 (anonymous function) toc.js:24 importParser.parseScript Parser.js:91 importParser.parse Parser.js:40 importParser.parseLink Parser.js:56 importParser.parse Parser.js:40 (anonymous function) boot.js:22 Loader.checkDone HTMLImports.js:267 Loader.tail HTMLImports.js:263 (anonymous function) HTMLImports.js:257 Loader.receive HTMLImports.js:253 (anonymous function) HTMLImports.js:232 (anonymous function) HTMLImports.js:437

jmesserly commented 10 years ago

related to this change: https://github.com/Polymer/TemplateBinding/commit/2c379a393ee73a80eda4ad3b38f6ec6a064c89c9

template.ref assumes getInstanceRoot returns a non-null value, which is only true if node was expanded from a template. My guess is that this line needs to have the if-check:

if (instanceRoot) ref = instanceRoot.querySelector('#' + refId);

and I'm guessing in your code somewhere, you have a ref="idName" where idName isn't found on the page. to see what's going on, you could put a breakpoint on https://github.com/Polymer/TemplateBinding/blob/2c379a393ee73a80eda4ad3b38f6ec6a064c89c9/src/TemplateBinding.js#L572 and see why it isn't finding the refId in the first query. Or try the patch above.

jyasskin commented 10 years ago

It's actually not finding a treeScope in the first query, so it never looks for the element at #refId.

rafaelw commented 10 years ago

John is right as usual. Fixed: https://github.com/Polymer/TemplateBinding/commit/5c5109f

jyasskin commented 10 years ago

I do have an element with the right id; it's just inside a custom element's template instead of at the document level. You can see this at http://jsbin.com/AmERAZIV/1/edit: if you take the <template bind="" id="hierarchy"> element and copy it to the document level, you'll see the hierarchy reappear.

@rafaelw's fix didn't fix this problem; it just hid it by removing the exception.

I can make this work with less duplication by moving the named template out of the template that produces shadow dom (http://jsbin.com/oGoporiC/1/edit), but if that's the intended interaction between template binding and custom elements, it should be documented.

rafaelw commented 10 years ago

Sorry, Jeffery. I think I've fixed it for good now: https://github.com/Polymer/TemplateBinding/commit/103de71

jyasskin commented 10 years ago

I tested your fix, and it appears to make my elements work. Thanks!