IE11: Fix incorrect node order with conditionals and text nodes #4650

deadem · 2025-02-10T14:13:54Z

There is an issue with using Element.contains in IE11. IE11 only partially supports contains: it works for Element nodes but does not support Text nodes. This can cause incorrect node ordering when conditionally rendering elements that include text nodes.

Minimal Repro:

import { render } from 'preact';
import { useEffect, useState } from 'preact/hooks';

export function App() {
  const [ showFirst, setShowFirst ] = useState(false);

  useEffect(function() {
    setTimeout(() => setShowFirst(true), 100);
  }, []);

  return (
    <div>
      {showFirst && <div>FIRST LINE</div>}
      plain text
      <div>LAST LINE</div>
    </div>
  );
}

Expected output:

FIRST LINE
plain text
LAST LINE

Actual output in IE11:

LAST LINE
FIRST LINE
plain text

…xt nodes

github-actions · 2025-02-10T14:14:30Z

📊 Tachometer Benchmark Results

Summary

duration

create10k: unsure 🔍 -1% - +1% (-10.89ms - +6.97ms)
^{preact-local vs preact-main}
filter-list: unsure 🔍 -1% - +2% (-0.25ms - +0.26ms)
^{preact-local vs preact-main}
hydrate1k: unsure 🔍 -2% - +1% (-1.45ms - +0.94ms)
^{preact-local vs preact-main}
many-updates: unsure 🔍 -2% - +1% (-0.42ms - +0.20ms)
^{preact-local vs preact-main}
replace1k: unsure 🔍 -4% - +1% (-2.57ms - +0.40ms)
^{preact-local vs preact-main}
text-update: unsure 🔍 -2% - +5% (-0.03ms - +0.10ms)
^{preact-local vs preact-main}
todo: unsure 🔍 -0% - +3% (-0.13ms - +0.97ms)
^{preact-local vs preact-main}
update10th1k: unsure 🔍 -4% - +4% (-1.31ms - +1.24ms)
^{preact-local vs preact-main}

usedJSHeapSize

create10k: unsure 🔍 +0% - +0% (+0.00ms - +0.00ms)
^{preact-local vs preact-main}
filter-list: unsure 🔍 -2% - +2% (-0.02ms - +0.03ms)
^{preact-local vs preact-main}
hydrate1k: unsure 🔍 -5% - +1% (-0.54ms - +0.15ms)
^{preact-local vs preact-main}
many-updates: unsure 🔍 -0% - +0% (-0.01ms - +0.01ms)
^{preact-local vs preact-main}
replace1k: unsure 🔍 -0% - +0% (-0.01ms - +0.01ms)
^{preact-local vs preact-main}
text-update: unsure 🔍 +0% - +0% (+0.00ms - +0.00ms)
^{preact-local vs preact-main}
todo: unsure 🔍 +0% - +0% (+0.00ms - +0.00ms)
^{preact-local vs preact-main}
update10th1k: unsure 🔍 -0% - +0% (-0.01ms - +0.01ms)
^{preact-local vs preact-main}

Results

create10k

Browser: chrome-headless
Sample size: 50
Built by: CI #4320
Commit: 9a06c29

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	974.30ms - 988.15ms	-	unsure 🔍 -1% - +1% -10.89ms - +6.97ms
preact-main	977.54ms - 988.83ms	unsure 🔍 -1% - +1% -6.97ms - +10.89ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	19.21ms - 19.21ms	-	unsure 🔍 +0% - +0% +0.00ms - +0.00ms
preact-main	19.20ms - 19.20ms	unsure 🔍 -0% - -0% -0.00ms - -0.00ms	-

filter-list

Browser: chrome-headless
Sample size: 50
Built by: CI #4320
Commit: 9a06c29

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	16.37ms - 16.83ms	-	unsure 🔍 -1% - +2% -0.25ms - +0.26ms
preact-main	16.48ms - 16.70ms	unsure 🔍 -2% - +1% -0.26ms - +0.25ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	1.54ms - 1.57ms	-	unsure 🔍 -2% - +2% -0.02ms - +0.03ms
preact-main	1.54ms - 1.57ms	unsure 🔍 -2% - +2% -0.03ms - +0.02ms	-

hydrate1k

Browser: chrome-headless
Sample size: 80
Built by: CI #4320
Commit: 9a06c29

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	76.92ms - 78.50ms	-	unsure 🔍 -2% - +1% -1.45ms - +0.94ms
preact-main	77.07ms - 78.86ms	unsure 🔍 -1% - +2% -0.94ms - +1.45ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	11.49ms - 11.94ms	-	unsure 🔍 -5% - +1% -0.54ms - +0.15ms
preact-main	11.65ms - 12.18ms	unsure 🔍 -1% - +5% -0.15ms - +0.54ms	-

many-updates

Browser: chrome-headless
Sample size: 50
Built by: CI #4320
Commit: 9a06c29

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	16.55ms - 16.90ms	-	unsure 🔍 -2% - +1% -0.42ms - +0.20ms
preact-main	16.58ms - 17.09ms	unsure 🔍 -1% - +2% -0.20ms - +0.42ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	3.75ms - 3.77ms	-	unsure 🔍 -0% - +0% -0.01ms - +0.01ms
preact-main	3.75ms - 3.77ms	unsure 🔍 -0% - +0% -0.01ms - +0.01ms	-

replace1k

Browser: chrome-headless
Sample size: 100
Built by: CI #4320
Commit: 9a06c29

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	68.51ms - 70.25ms	-	unsure 🔍 -4% - +1% -2.57ms - +0.40ms
preact-main	69.26ms - 71.66ms	unsure 🔍 -1% - +4% -0.40ms - +2.57ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	2.97ms - 2.98ms	-	unsure 🔍 -0% - +0% -0.01ms - +0.01ms
preact-main	2.97ms - 2.99ms	unsure 🔍 -0% - +0% -0.01ms - +0.01ms	-

run-warmup-0

Version	Avg time	vs preact-local	vs preact-main
preact-local	30.64ms - 31.23ms	-	unsure 🔍 -1% - +1% -0.42ms - +0.39ms
preact-main	30.67ms - 31.23ms	unsure 🔍 -1% - +1% -0.39ms - +0.42ms	-

run-warmup-1

Version	Avg time	vs preact-local	vs preact-main
preact-local	35.43ms - 36.29ms	-	unsure 🔍 -3% - +1% -0.91ms - +0.33ms
preact-main	35.71ms - 36.60ms	unsure 🔍 -1% - +3% -0.33ms - +0.91ms	-

run-warmup-2

Version	Avg time	vs preact-local	vs preact-main
preact-local	27.20ms - 27.59ms	-	unsure 🔍 -1% - +1% -0.36ms - +0.19ms
preact-main	27.28ms - 27.67ms	unsure 🔍 -1% - +1% -0.19ms - +0.36ms	-

run-warmup-3

Version	Avg time	vs preact-local	vs preact-main
preact-local	27.10ms - 28.28ms	-	unsure 🔍 -5% - +1% -1.34ms - +0.40ms
preact-main	27.52ms - 28.80ms	unsure 🔍 -1% - +5% -0.40ms - +1.34ms	-

run-warmup-4

Version	Avg time	vs preact-local	vs preact-main
preact-local	25.25ms - 26.60ms	-	unsure 🔍 -2% - +5% -0.62ms - +1.26ms
preact-main	24.95ms - 26.26ms	unsure 🔍 -5% - +2% -1.26ms - +0.62ms	-

run-final

Version	Avg time	vs preact-local	vs preact-main
preact-local	21.34ms - 21.75ms	-	unsure 🔍 -2% - +0% -0.52ms - +0.10ms
preact-main	21.53ms - 21.98ms	unsure 🔍 -0% - +2% -0.10ms - +0.52ms	-

text-update

Browser: chrome-headless
Sample size: 200
Built by: CI #4320
Commit: 9a06c29

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	2.03ms - 2.13ms	-	unsure 🔍 -2% - +5% -0.03ms - +0.10ms
preact-main	2.00ms - 2.09ms	unsure 🔍 -5% - +2% -0.10ms - +0.03ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	1.12ms - 1.12ms	-	unsure 🔍 +0% - +0% +0.00ms - +0.00ms
preact-main	1.11ms - 1.11ms	unsure 🔍 -0% - -0% -0.00ms - -0.00ms	-

todo

Browser: chrome-headless
Sample size: 50
Built by: CI #4320
Commit: 9a06c29

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	33.79ms - 34.84ms	-	unsure 🔍 -0% - +3% -0.13ms - +0.97ms
preact-main	33.72ms - 34.06ms	unsure 🔍 -3% - +0% -0.97ms - +0.13ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	1.22ms - 1.23ms	-	unsure 🔍 +0% - +0% +0.00ms - +0.00ms
preact-main	1.22ms - 1.22ms	unsure 🔍 -0% - -0% -0.00ms - -0.00ms	-

update10th1k

Browser: chrome-headless
Sample size: 50
Built by: CI #4320
Commit: 9a06c29

duration

Version	Avg time	vs preact-local	vs preact-main
preact-local	32.35ms - 34.13ms	-	unsure 🔍 -4% - +4% -1.31ms - +1.24ms
preact-main	32.36ms - 34.18ms	unsure 🔍 -4% - +4% -1.24ms - +1.31ms	-

usedJSHeapSize

Version	Avg time	vs preact-local	vs preact-main
preact-local	2.93ms - 2.95ms	-	unsure 🔍 -0% - +0% -0.01ms - +0.01ms
preact-main	2.93ms - 2.95ms	unsure 🔍 -0% - +0% -0.01ms - +0.01ms	-

_{tachometer-reporter-action v2 for CI}

deadem · 2025-02-10T14:48:30Z

Wow! There is already a hack:

preact/compat/src/portals.js

Line 42 in 3ff5f50

contains: () => true,

coveralls · 2025-02-13T02:44:44Z

coverage: 99.62% (+0.002%) from 99.618%
when pulling 9a06c29 on deadem:ie-contains-fix
into 3ff5f50 on preactjs:main.

JoviDeCroock · 2025-02-14T11:48:11Z

src/diff/children.js

 		return oldDom;
 	} else if (parentVNode._dom != oldDom) {
-		if (oldDom && parentVNode.type && !parentDom.contains(oldDom)) {
+		if (oldDom && parentVNode.type && !contains(parentDom, oldDom)) {


I think we could solve this a lot simpler, similar to #4666 - alternatively, we could check whether oldDom is in fact a DOM-node rather than a text-node.

JoviDeCroock · 2025-02-14T11:48:44Z

compat/src/portals.js

 			parentNode: container,
 			childNodes: [],
-			contains: () => true,
+			isPortal: true,


This is an unsafe change as other faux roots could be using this, also contains is not a hack, it's a DOM built-in

Fix: Ensure IE11 compatibility for contains function by handling te…

b1164cf

…xt nodes

* add special case for portals handling

9a06c29

JoviDeCroock reviewed Feb 14, 2025

View reviewed changes

Uh oh!

IE11: Fix incorrect node order with conditionals and text nodes #4650

Are you sure you want to change the base?

IE11: Fix incorrect node order with conditionals and text nodes #4650

Uh oh!

Conversation

deadem commented Feb 10, 2025

Uh oh!

github-actions bot commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Tachometer Benchmark Results

Summary

duration

usedJSHeapSize

Results

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

run-warmup-0

run-warmup-1

run-warmup-2

run-warmup-3

run-warmup-4

run-final

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

Uh oh!

deadem commented Feb 10, 2025

Uh oh!

coveralls commented Feb 13, 2025

Uh oh!

JoviDeCroock Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoviDeCroock Feb 14, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Feb 10, 2025 •

edited

Loading

JoviDeCroock Feb 14, 2025 •

edited

Loading