Skip to content

Update polymer children using polymer api#5948

Closed
jimfb wants to merge 1 commit intofacebook:masterfrom
jimfb:polymer-children
Closed

Update polymer children using polymer api#5948
jimfb wants to merge 1 commit intofacebook:masterfrom
jimfb:polymer-children

Conversation

@jimfb
Copy link
Contributor

@jimfb jimfb commented Jan 30, 2016

For web components using polymer, we sometimes screw up the updates of children, because polymer uses a shady dom. This PR will use the polymer DOM api to update children IFF the user is using polymer, thereby ensuring that all DOM child operations happen correctly.

@jimfb
Copy link
Contributor Author

jimfb commented Jan 30, 2016

@spicyj @sebmarkbage

@sophiebits
Copy link
Collaborator

Polymer is supposed to be completely transparent. If you're loading Polymer before React and React doesn't work, it's a Polymer bug.

@sophiebits
Copy link
Collaborator

ex: #1263 (comment)

@jimfb
Copy link
Contributor Author

jimfb commented Jan 30, 2016

@spicyj: shady dom, not shadow dom.

Reference: https://www.polymer-project.org/1.0/articles/shadydom.html#shady-dom-is-born

@sebmarkbage
Copy link
Contributor

This feels very shady indeed.

  1. The hook is in the wrong place since it rechecks in the hot path for everyone.
  2. It checks a global but not a module system based hook which is unfortunate since you can never use it with a module system.
  3. It effectively claims the Polymer name since browsers can no longer add it to the global namespace without breaking the web. Usually it works because pages that rely on Polymer would shadow such a name. Maybe that's too late any donno.
  4. It adds a special hook to a particular library, unfairly favoring this particular implementation rather than a different web components polyfill. If there was a more generic and standardized hook that was more generic, this wouldn't feel as bad.
  5. Polymer isn't stable and popular enough to warrant this kind of special casing. jQuery, Immutable-js, Lodash lazy collections etc. would probably be more interesting to special case if we did that for anything.

@gaearon
Copy link
Collaborator

gaearon commented Mar 27, 2016

Closing per #5948 (comment). We can open a separate issue if this is something we want to track.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants