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

Can we notify Polymer about changes once entire patch applied? #29

Open
miyconst opened this issue Aug 15, 2016 · 8 comments
Open

Can we notify Polymer about changes once entire patch applied? #29

miyconst opened this issue Aug 15, 2016 · 8 comments

Comments

@miyconst
Copy link

It is very bug prone when Polymer observers are hit in between of patch items.

cc @warpech, @tomalec

@warpech
Copy link
Contributor

warpech commented Aug 16, 2016

I'd rather avoid it, because then the client side code becomes aware of Puppet.

Can you give any examples when its really unavoidable?

As a workaround, perhaps you can have a property Updated: 0 that is incremented on the server and set the observer on it?

@miyconst
Copy link
Author

I'd rather avoid it, because then the client side code becomes aware of Puppet.

Just opposite. It should be hidden into the puppet-polymer-client. Currently we have to be aware that the view-model can get into inconsistent state at any moment and care about it in every web component.

The last case which raised this issue:

<palma-d3-matrix rows="{{model.Rows}}" cols="{{model.Cols}}" cells="{{model.Cells}}"></palma-d3-matrix>

The Rows, Cols, and Cells arrays are updated on certain user action. Now the problem is that palma-d3-matrix observes changes on those values and expect to have cells for existing rows and cols. What happens in reality is that polymer notifies palma-d3-matrix about Cells change before the Rows and Cols have actually been updated.

Even more weird staff happening when partial.Html property is involved.

@warpech
Copy link
Contributor

warpech commented Aug 16, 2016

What version of Starcounter and puppet-polymer-client are you using? Current version uses notifyPath (see https://github.com/PuppetJs/puppet-polymer-client/blob/gh-pages/puppet-client.html#L467) after all of the sequence operations are actually applied.

@miyconst
Copy link
Author

You are right. After some more digging and investigation I found that my issue is caused by the following:

  • Client receives a patch to replace a partial.
  • PuppetJs applies view-model changes.
  • puppet-polymer-client notifies Polymer about the changes.
  • Polymer applies the view-model changes to the old HTML.
  • Exception happens here, as the old HTML is not designed for the new view-model.
  • starcounter-include removes old HTML.
  • starcounter-include loads new partial.

@miyconst
Copy link
Author

I have created a simplest possible example in my Playground.

In the example I have two partials with { "Date": "" } property defined, loading one after another will apply the new value to old HTML first and only then replace the entire partial.

Steps to reproduce:

  1. Clone the repository.
  2. Start Playground app.
  3. Open http://localhost:8080/playground.
  4. Open Chrome Console.
  5. Click [Load date page] button.
  6. Click [Load sub page] button.
  7. The DatePage partial will throw an exception because the value from SubPage was applied to it.

Replace console.error with debugger if you want to stop for debugging.

@tomalec
Copy link
Member

tomalec commented Aug 29, 2016

For the first look, it seems to me like a bug (missing scenario coverage) in starcounter-include (juicy-html). Could you provide an example of a patch received by client?

@miyconst
Copy link
Author

@tomalec here is the patch:

[
  {
    "op": "replace",
    "path": "/_ver#s",
    "value": 3
  },
  {
    "op": "test",
    "path": "/_ver#c$",
    "value": 2
  },
  {
    "op": "replace",
    "path": "/SubPage",
    "value": {
      "Playground": {
        "Html": "/Playground/SubPage.html",
        "Date": "SubPage",
        "Items": [ "i7-4790k", "i7-5820k", "i7-6700k" ],
        "SelectedItem$": "",
        "Details": { },
        "Remove$": 0,
        "Add$": 0,
        "Message": ""
      }
    }
  },
  {
    "op": "replace",
    "path": "/LoadSubPage$",
    "value": 0
  }
]

Can you look at my example? It is tiny and easy to understand.

@warpech
Copy link
Contributor

warpech commented Oct 2, 2017

Let's look at this when Polymer 2.0 integration is tested in Starcounter 2.4.

@warpech warpech added this to the Palindrom stability milestone Apr 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants