Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[0.9] Annotated computed properties don't work on autobinding template #1500

Closed
arthurevans opened this issue May 7, 2015 · 14 comments
Closed
Assignees

Comments

@arthurevans
Copy link

Not sure if this a bug or not, but because we rely on A. C. P. to replace expressions, it would be nice to be able to use them on a dom-bind template.

Currently, it's failing with the error:

"TypeError: Cannot read property 'apply' of undefined
    at function-test.Polymer.Base.extend._annotatedComputationEffect (https://rawgit.com/polymer/polymer/0.8-preview/src/lib/bind/effects.html:73:38)

Repro:

http://jsbin.com/hijise/1/edit?html,console,output

@arthurevans
Copy link
Author

Found this while responding to #1499.

@arthurevans arthurevans changed the title Annotated computed properties don't work on autobinding template [0.9] Annotated computed properties don't work on autobinding template May 7, 2015
@jlg7
Copy link

jlg7 commented May 7, 2015

@arthurevans - is inlineFunction supposed to be defined? I haven't ran into this issue and wanted to make sure there is no edge case!

Thank you!

@arthurevans
Copy link
Author

agh. Sorry, apparently I edited that jsbin after posting. Sigh.

The sample I intended to post was here:

http://output.jsbin.com/xeweza/2/edit?html,output

... but it gets stranger. The dom-if is failing when I'm in edit more in jsbin, but if I preview the page (remove the edit?... from the URL), it appears to work correctly.

@kevinpschaaf
Copy link
Member

Good catch, Arthur. Root cause was that dom-bind was being registered before dom-repeat, and this could cause the dom-bind to stamp the dom-repeat as an unknown element which then gets upgraded later (without ability to determine the correct "dataHost" for calling computed functions on).

The flakiness there was pointing to a race condition re: HTML imports loading vs. elements being parsed in the main document. For example, I couldn't reproduce this locally because apparently the imports were loading fast enough to be parsed & executed before the elements in the main document, but on jsBin with higher latency, the elements were parsed and then upgraded synchronously in import order, causing the dom-bind to be stamped before dom-repeat was registered.

Moving the registration of dom-bind to be after dom-repeat should solve your use case, but we'll need to consider whether this is going to cause problems for other elements. For example x-repeat before (or after) x-if, or dom-repeat before paper-button...

@kevinpschaaf
Copy link
Member

271a9e3 should fix the use case highlighted, will keep issue open to consider other possible use cases that could fail before closing.

@jlg7
Copy link

jlg7 commented May 10, 2015

I ran across some confusing behavior that may be appropriate to mention here. Are annotated computed properties taking the item argument supported inside of a dom-if nested in a dom-repeat?

Example:

<link rel="import" href="bower_components/polymer/polymer.html">

<dom-module id="sample-element">
  <link rel="stylesheet" type="css" href="sample-element.css">
  <template>
   <template is="dom-repeat" items="{{data}}" as="i">
      <div>
        <span>Outside of dom-if:</span>
        <span>{{computeFancyName(i)}}</span>
      </div>
      <template is="dom-if" if="{{showItem(i)}}">
          <div>
            <span>Computed Inside of dom-if for item's named "A":</span>
            <span>{{computeFancyName(i)}}</span>
          </div>
          <!-- uncomment below to make the below div to see the annotated computed property place the value in the dom -->
          <!-- <div>
            <span>Not computed Inside of dom-if for item's named "A":</span>
            <span>{{i.name}}</span>
          </div> -->
      </template>

   </template>
  </template>
</dom-module>
<script>
    Polymer({

      is:'sample-element',
      properties:{
        data:{
          type: Array,
          value:function(){
            return [
              {
                "id":1,
                "name":"A",
                "data":[
                  {
                  "id":1,
                  "name":"Baby A"
                  }
                ]
              },
              {
                "id":2,
                "name":"B"
              },
              {
                "id":3,
                "name":"C"
              }
              ];
          }
        }
      },
      showItem:function(item){
          return item.name ===  "A";
      },
      computeFancyName:function(item){
        return "*"+item.name+"*";
      }
    });
</script>

The annotated computed property function is being executed in the dom-if scope but the computed value is not placed into the dom. Even weirder is that it works when the div that is currently commented out in the example is used.

@arthurevans
Copy link
Author

@jongeho1 That definitely looks like another corner case.

@kevinpschaaf I put this example a jsbin here: http://jsbin.com/qefuqe/1/edit?html,console,output

Interesting to note: there's a log message for all combinations:

"TypeError: Cannot set property 'display' of undefined
at dom-if.Polymer._hideChildren (https://rawgit.com/polymer/polymer/0.8-preview/src/lib/template/dom-if.html:148:27)

It seems to be the presence of the second, non-computed binding (rather than the presence of absence of commented-out blocks, for example) that causes the computed property to display correctly inside the dom-if.

@jlg7
Copy link

jlg7 commented May 10, 2015

@arthurevans - thank you! That is definitely what I was trying to articulate in terms of behavior.

TypeError: Cannot set property 'display' of undefined seems unrelated to the annotated computed property inside the dom-if rather more cosmetic in nature. If the template stamper/render ignored/filtered comment nodes, then this would go away I'd imagine.

@mcarey1590
Copy link

I am having the same issue with a computed property function never being called inside a dom-repeat using two arguments.

<template is="dom-repeat" items="{{tableData}}">
         <tr>
              <template is="dom-repeat" items="{{tableDataLookupKeys}}" as="column">
                        <td><span>{{_computeTableColumn(item, column)}}</span></td>
               </template>
          </tr>
</template>

however, only using one argument

<td><span>{{_computeTableColumn(column)}}</span></td>

will call the function

@kevinpschaaf
Copy link
Member

Confirmed the issue, we'll get this fixed.

@kevinpschaaf
Copy link
Member

@jongeho1 @mcarey1590 The secondary issue mentioned in #1500 (comment) should now be fixed per #1514.

@jlg7
Copy link

jlg7 commented May 12, 2015

@kevinpschaaf @arthurevans TYVM! Will try it out!

@mcarey1590
Copy link

@kevinpschaaf so far so good, thank you

@kevinpschaaf
Copy link
Member

Solution to general ordering problem is to make dom-bind to wait for HTMLImports.whenReady (when on polyfill) or window.load.

@kevinpschaaf kevinpschaaf added p1 and removed 0.8 labels May 21, 2015
@kevinpschaaf kevinpschaaf self-assigned this May 21, 2015
kevinpschaaf added a commit to webcomponents/webcomponentsjs that referenced this issue May 22, 2015
sorvell pushed a commit that referenced this issue May 23, 2015
Wait until imports resolve to stamp. Fixes #1500
kevinpschaaf added a commit to webcomponents/webcomponentsjs that referenced this issue May 23, 2015
Part of fix for Polymer/polymer#1500

Use existing decorate fn.

Move children into content unconditionally, so this happens for imperatively-created templates also, to support innerHTML setting before DOMContentLoaded.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants