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

Legacy support issue in IE11 on hydration #7723

Closed
Tal500 opened this issue Jul 25, 2022 · 2 comments · Fixed by #7724
Closed

Legacy support issue in IE11 on hydration #7723

Tal500 opened this issue Jul 25, 2022 · 2 comments · Fixed by #7724

Comments

@Tal500
Copy link
Contributor

Tal500 commented Jul 25, 2022

Describe the bug

Commit 04bc37d of Svelte breaks hydration in IE11. Elements seems to be "shuffled" after hydration.
Indeed, in the effort of handling legacy support of SvelteKit (sveltejs/kit#12), specifically on the PR draft sveltejs/kit#2745 the author of PR describe this problem as something that wasn't fixed:

While hydration in IE11 there is a bug that makes all elements "shuffle" on IE11 (it is better to see this). Was not trying to figure out this problem yet.

The reason it happen is very simple. In dom.ts, the author used Node.parentElement:

if ((target.actual_end_child === undefined) || ((target.actual_end_child !== null) && (target.actual_end_child.parentElement !== target))) {

But as you can see in MDN,
תמונה

IE11 and some other legacy browsers don't support Node.parentElement if the current node isn't Element.

The solution is very simple - just use Node.parentNode instead, long description for an easy fix. I will send PR for this soon.

An alternative to this simple method is to force to use a polyfill that implement Node.parentElement via Node.parentNode, but it is not common (didn't find it on polyfill.io, or any other common resource). I did find though some implementation, which is for short:

function implementation () {
    return this.parentNode instanceof Element ? this.parentNode : null;
}

But I don't think it's a good idea to force all the legacy users to implement this non-standard polyfill, when the other solution is very easy.

Reproduction

The reproduction is easy - simply by the old good repo sapper-ie.

If you will use the same version of packages that are currently defined in this repo (by npm ci), you'd find that this sapper example works as it should:
תמונה

However, if you install an updated version of svelte in this repo (every version >= 3.38.3, since every such version includes 04bc37d), you'll see that elements are shuffled:
תמונה

Logs

No response

System Info

Output of `npx envinfo --system --npmPackages svelte,rollup,webpack --binaries --browsers` :

  System:
    OS: Windows 10 10.0.19044
    CPU: (4) x64 Intel(R) Core(TM) i7-6500U CPU @ 2.50GHz
    Memory: 4.37 GB / 15.49 GB
  Binaries:
    Node: 18.3.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.18 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 8.14.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1266.0), Chromium (103.0.1264.71)
    Internet Explorer: 11.0.19041.1566
  npmPackages:
    rollup: ^1.27.14 => 1.27.14


### Severity

annoyance
@Conduitry
Copy link
Member

This should be fixed in 3.50.0.

@Tal500
Copy link
Contributor Author

Tal500 commented Sep 2, 2022

This should be fixed in 3.50.0.

Releasing a version on Friday... that's brave of you😁

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

Successfully merging a pull request may close this issue.

2 participants