Skip to content
This repository has been archived by the owner on Sep 25, 2021. It is now read-only.

Improve compatibility with webcomponents.js (connectedCallback) #337

Closed
wants to merge 1 commit into from
Closed

Improve compatibility with webcomponents.js (connectedCallback) #337

wants to merge 1 commit into from

Conversation

denisw
Copy link
Contributor

@denisw denisw commented Dec 15, 2017

Background

Custom Elements are a great fit for Turbolinks (and are rightfully mentioned in the Turbolinks README). Unfortunately, more often than not it's still required to use the webcomponents.js Custom Elements polyfill to support browsers other than Chrome and Safari 11.

This, in itself, is not much of a problem. However, I noticed that on browsers that need to fall back to the polyfill, my custom elements' connectedCallback methods are not called after following a link, breaking them all. 馃槥

The Problem

When visiting a new page, Turbolinks currently replaces the body by setting document.body:

  assignNewBody: ->
    document.body = @newBody

However, the polyfill cannot detect document.body changes (webcomponents/custom-elements#126). So it will not be able to walk the DOM and call the connectedCallbackmethods of all custom elements.

The Solution

We can make the webcomponents.js polyfill aware of the body change by simply using Element.prototype.replaceChild instead:

  assignNewBody: ->
    document.documentElement.replaceChild(@newBody, document.body)

This version (implemented by this PR) is totally equivalent to setting document.body, but replaceChild is patched by the polyfill, so connectedCallback works as expected.

There are no disadvantages to this change. As replaceChild is supported on all browsers including IE back to version 6, no new browser incompatibilities are introduced.

Previously, Turbolinks replaced the document body by settting
document.body. However, the reference Custom Elements polyfill
[1] does not patch the document.body property, the connectedCallback
of any custom elements in the new body will never be called.

By using document.documentElement.replaceChild(), we make it possible
for the Custom Elements polyfill to detect that new elements were
connected do the DOM.

[1]: https://github.com/webcomponents/custom-elements
@denisw denisw changed the title Improve compatibility with webcomponentsjs (connectedCallback) Improve compatibility with webcomponents.js (connectedCallback) Dec 15, 2017
@javan
Copy link
Contributor

javan commented Dec 19, 2017

Thanks for the heads up! Your patch to fix the the polyfill seems like the best overall solution: webcomponents/custom-elements#127.

@javan javan closed this Dec 19, 2017
@seanpdoyle
Copy link
Contributor

According to webcomponents/custom-elements#127 (comment), @denisw 's change was merged, reverted, then re-opened as webcomponents/custom-elements#134.

Unfortunately, fixing this WebComponents-side seems to be blocked.

@javan is this proposed change worth reconsidering?

@sstephenson
Copy link
Contributor

We'll need to test it carefully, of course, but I don't see any issue with using replaceChild.

@sstephenson sstephenson reopened this Oct 17, 2018
@javan
Copy link
Contributor

javan commented Oct 17, 2018

+1, there's a new replaceElementWithElement() helper that was added after this PR was opened:

replaceElementWithElement = (fromElement, toElement) ->
if parentElement = fromElement.parentNode
parentElement.replaceChild(toElement, fromElement)

We should probably use it instead of the current implementation for consistency.

seanpdoyle added a commit to seanpdoyle/turbolinks that referenced this pull request Oct 18, 2018
Closes turbolinks#337

Background
---

[Custom Elements][mdn] are a great fit for Turbolinks. Unfortunately,
more often than not it's still required to use the `webcomponents.js`
Custom Elements [polyfill] to support browsers other than Chrome and
Safari 11.

This, in itself, is not much of a problem. However, I noticed that on
browsers that need to fall back to the polyfill, my custom elements'
`connectedCallback` methods are not called after following a link,
breaking them all.

Problem
---

When visiting a new page, Turbolinks currently replaces the body by
setting `document.body`:

```coffee
assignNewBody: ->
  document.body = @newbody
```

However, the polyfill cannot detect document.body changes
(webcomponents/custom-elements#126). So it will not be able to walk the
DOM and call the `connectedCallback` methods of all custom elements.

Solution
---

Previously, `Turbolinks` replaced the document body by setting
`document.body`. However, the reference Custom Elements [polyfill]
does not patch the `document.body` property, the `connectedCallback` of
any custom elements in the new body will never be called.

By using `document.documentElement.replaceChild()`, we make it possible
for the Custom Elements polyfill to detect that new elements were
connected do the DOM.

[mdn]: https://developer.mozilla.org/en-US/docs/Web/Web_Components/Custom_Elements
[polyfill]: https://github.com/webcomponents/custom-elements

Co-authored-by: Sean Doyle <sean.p.doyle24@gmail.com>
Co-authored-by: Denis Washington <denis@denisw.de>
@seanpdoyle
Copy link
Contributor

seanpdoyle commented Oct 18, 2018

@sstephenson @javan I've opened #431 so that the CI test suite can run on then change, and on the off-change that this PR stays inactive.

If it'd be better for us to centralize the work to this PR, I'll gladly close #431.

I've credited @denisw as a co-author on that commit.

@sstephenson
Copy link
Contributor

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

Successfully merging this pull request may close these issues.

None yet

4 participants