Skip to content

Commit fb5bbda

Browse files
JoviDeCroockJoviDeCroock
authored
Forwardport v10.x fixes since 2026-03-25 (#5089)
* Fix incorrect DOM order with conditional ContextProvider and inner keys (#5065) (#5067) When a VNode is moved via INSERT_VNODE during diffChildren, its old _dom pointer becomes a stale positional reference. Nested diffs that call getDomSibling would traverse the old VNode tree and find this stale _dom, causing subsequent DOM insertions at the wrong position. Clear the old VNode's _dom after insert so getDomSibling skips moved VNodes and finds the correct insertion point. * Create a unique event-clock for each Preact instance on a page. (#5068) * Fix migrations when we have defaultValue or value on a textarea (#5081) * Add CODEOWNERS for GitHub configuration (#5088) * chore: remove unused Fragment import in forwardported test
1 parent a31df28 commit fb5bbda

7 files changed

Lines changed: 269 additions & 10 deletions

File tree

‎.github/CODEOWNERS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# Request review for GitHub configuration changes.
2+
.github/ @JoviDeCroock

‎src/diff/children.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,14 @@ export function diffChildren(
128128
let shouldPlace = childVNode._flags & INSERT_VNODE;
129129
if (shouldPlace || oldVNode._children === childVNode._children) {
130130
oldDom = insert(childVNode, oldDom, parentDom, shouldPlace);
131+
132+
// When a matched VNode is physically moved via INSERT_VNODE, its old
133+
// _dom pointer becomes a stale positional reference. Clear it so that
134+
// getDomSibling (called from nested diffs) won't return this stale
135+
// reference and mis-place subsequent DOM nodes. See #5065.
136+
if (shouldPlace && oldVNode._dom) {
137+
oldVNode._dom = NULL;
138+
}
131139
} else if (typeof childVNode.type == 'function' && result !== UNDEFINED) {
132140
oldDom = result;
133141
} else if (newDom) {

‎src/diff/index.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,10 @@ function diffElementNodes(
528528
}
529529
} else {
530530
// If excessDomChildren was not null, repopulate it with the current element's children:
531-
excessDomChildren = excessDomChildren && slice.call(dom.childNodes);
531+
excessDomChildren =
532+
nodeType == 'textarea' && newProps.defaultValue != NULL
533+
? NULL
534+
: excessDomChildren && slice.call(dom.childNodes);
532535

533536
// If we are in a situation where we are not hydrating but are using
534537
// existing DOM (e.g. replaceNode) we should read the existing DOM
@@ -625,7 +628,7 @@ function diffElementNodes(
625628
}
626629

627630
// As above, don't diff props during hydration
628-
if (!isHydrating) {
631+
if (!isHydrating || nodeType == 'textarea') {
629632
i = 'value';
630633
if (nodeType == 'progress' && inputValue == NULL) {
631634
dom.removeAttribute('value');

‎src/diff/props.js‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,17 @@
11
import { NULL, SVG_NAMESPACE } from '../constants';
22
import options from '../options';
33

4+
// Per-instance unique key for event clock stamps. Each Preact copy on the page
5+
// gets its own random suffix so that `_dispatched` / `_attached` properties on
6+
// shared event objects and handler functions cannot collide across instances.
7+
// ~1 in 60M collision odds - if you have that many praect versions on the page,
8+
// you deserve some weird bugs.
9+
// In 11 we can replace this with a
10+
// Symbol
11+
let _id = Math.random().toString(8),
12+
EVENT_DISPATCHED = '__d' + _id,
13+
EVENT_ATTACHED = '__a' + _id;
14+
415
function setStyle(style, key, value) {
516
if (key[0] == '-') {
617
style.setProperty(key, value == NULL ? '' : value);
@@ -74,14 +85,14 @@ export function setProperty(dom, name, value, oldValue, namespace) {
7485

7586
if (value) {
7687
if (!oldValue) {
77-
value._attached = eventClock;
88+
value[EVENT_ATTACHED] = eventClock;
7889
dom.addEventListener(
7990
name,
8091
useCapture ? eventProxyCapture : eventProxy,
8192
useCapture
8293
);
8394
} else {
84-
value._attached = oldValue._attached;
95+
value[EVENT_ATTACHED] = oldValue[EVENT_ATTACHED];
8596
}
8697
} else {
8798
dom.removeEventListener(
@@ -150,13 +161,13 @@ function createEventProxy(useCapture) {
150161
return function (e) {
151162
if (this._listeners) {
152163
const eventHandler = this._listeners[e.type + useCapture];
153-
if (e._dispatched == NULL) {
154-
e._dispatched = eventClock++;
164+
if (e[EVENT_DISPATCHED] == NULL) {
165+
e[EVENT_DISPATCHED] = eventClock++;
155166

156-
// When `e._dispatched` is smaller than the time when the targeted event
167+
// When `e[EVENT_DISPATCHED]` is smaller than the time when the targeted event
157168
// handler was attached we know we have bubbled up to an element that was added
158169
// during patching the DOM.
159-
} else if (e._dispatched < eventHandler._attached) {
170+
} else if (e[EVENT_DISPATCHED] < eventHandler[EVENT_ATTACHED]) {
160171
return;
161172
}
162173
return eventHandler(options.event ? options.event(e) : e);

‎src/internal.d.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,9 @@ export interface PreactElement extends preact.ContainerNode {
127127
}
128128

129129
export interface PreactEvent extends Event {
130-
_dispatched?: number;
130+
// Keyed by a per-instance unique string (e.g. `__dXXXXX`) so that
131+
// multiple Preact copies on the same page don't share event clock stamps.
132+
[key: string]: any;
131133
}
132134

133135
// We use the `current` property to differentiate between the two kinds of Refs so

‎test/browser/hydrate.test.jsx‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,20 @@ describe('hydrate()', () => {
6969
expect(scratch.firstChild.value).to.equal('foo');
7070
});
7171

72+
// Test for preactjs/preact#5080
73+
it('should respect textarea defaultValue in hydrate', () => {
74+
scratch.innerHTML = '<textarea>foo</textarea>';
75+
hydrate(<textarea defaultValue="foo" />, scratch);
76+
expect(scratch.firstChild.value).to.equal('foo');
77+
expect(scratch.firstChild.defaultValue).to.equal('foo');
78+
});
79+
80+
it('should respect textarea value in hydrate', () => {
81+
scratch.innerHTML = '<textarea>foo</textarea>';
82+
hydrate(<textarea value="foo" />, scratch);
83+
expect(scratch.firstChild.value).to.equal('foo');
84+
});
85+
7286
it('should respect defaultChecked in hydrate', () => {
7387
scratch.innerHTML = '<input checked="true">';
7488
hydrate(<input defaultChecked />, scratch);

‎test/browser/keys.test.jsx‎

Lines changed: 220 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { createElement, Component, render, createRef } from 'preact';
1+
import {
2+
createElement,
3+
Component,
4+
render,
5+
createRef,
6+
createContext
7+
} from 'preact';
28
import { setupRerender } from 'preact/test-utils';
39
import { setupScratch, teardown } from '../_util/helpers';
410
import { logCall, clearLog, getLog } from '../_util/logCall';
@@ -1166,4 +1172,217 @@ describe('keys', () => {
11661172
'<ol>2345.appendChild(<li>6)'
11671173
]);
11681174
});
1175+
1176+
// https://github.com/preactjs/preact/issues/5065
1177+
it('should maintain correct DOM order with conditional ContextProvider and inner keys', () => {
1178+
const Ctx = createContext(null);
1179+
1180+
class Label extends Component {
1181+
render({ todo }) {
1182+
return <li key={todo.text}>{todo.text}</li>;
1183+
}
1184+
}
1185+
1186+
class App extends Component {
1187+
render() {
1188+
const { todos } = this.props;
1189+
return (
1190+
<ul>
1191+
{todos.map((todo, index) =>
1192+
todo.text === '1' || todo.text === '2' || todo.text === '3' ? (
1193+
<Ctx.Provider value={null}>
1194+
<Label todo={todo} index={index} />
1195+
</Ctx.Provider>
1196+
) : (
1197+
<Label todo={todo} index={index} />
1198+
)
1199+
)}
1200+
</ul>
1201+
);
1202+
}
1203+
}
1204+
1205+
// Initial: 3 collapsed rows
1206+
render(
1207+
<App todos={[{ text: '1' }, { text: '2' }, { text: '3' }]} />,
1208+
scratch
1209+
);
1210+
expect(scratch.querySelector('ul').textContent).to.equal('123');
1211+
1212+
// Expand row 1: adds sub-items 1A, 1B, 1C (not wrapped in Provider)
1213+
render(
1214+
<App
1215+
todos={[
1216+
{ text: '1' },
1217+
{ text: '1A' },
1218+
{ text: '1B' },
1219+
{ text: '1C' },
1220+
{ text: '2' },
1221+
{ text: '3' }
1222+
]}
1223+
/>,
1224+
scratch
1225+
);
1226+
expect(scratch.querySelector('ul').textContent).to.equal('11A1B1C23');
1227+
1228+
// Expand row 2: adds sub-items 2A, 2B, 2C
1229+
render(
1230+
<App
1231+
todos={[
1232+
{ text: '1' },
1233+
{ text: '1A' },
1234+
{ text: '1B' },
1235+
{ text: '1C' },
1236+
{ text: '2' },
1237+
{ text: '2A' },
1238+
{ text: '2B' },
1239+
{ text: '2C' },
1240+
{ text: '3' }
1241+
]}
1242+
/>,
1243+
scratch
1244+
);
1245+
expect(scratch.querySelector('ul').textContent).to.equal('11A1B1C22A2B2C3');
1246+
1247+
// Collapse row 1: removes 1A, 1B, 1C
1248+
render(
1249+
<App
1250+
todos={[
1251+
{ text: '1' },
1252+
{ text: '2' },
1253+
{ text: '2A' },
1254+
{ text: '2B' },
1255+
{ text: '2C' },
1256+
{ text: '3' }
1257+
]}
1258+
/>,
1259+
scratch
1260+
);
1261+
expect(scratch.querySelector('ul').textContent).to.equal('122A2B2C3');
1262+
});
1263+
1264+
// https://github.com/preactjs/preact/issues/5065
1265+
it('should maintain correct DOM order when collapsing row 2 with conditional Provider', () => {
1266+
const Ctx = createContext(null);
1267+
1268+
class Label extends Component {
1269+
render({ todo }) {
1270+
return <li key={todo.text}>{todo.text}</li>;
1271+
}
1272+
}
1273+
1274+
class App extends Component {
1275+
render() {
1276+
const { todos } = this.props;
1277+
return (
1278+
<ul>
1279+
{todos.map((todo, index) =>
1280+
todo.text === '1' || todo.text === '2' || todo.text === '3' ? (
1281+
<Ctx.Provider value={null}>
1282+
<Label todo={todo} index={index} />
1283+
</Ctx.Provider>
1284+
) : (
1285+
<Label todo={todo} index={index} />
1286+
)
1287+
)}
1288+
</ul>
1289+
);
1290+
}
1291+
}
1292+
1293+
// Both rows expanded
1294+
render(
1295+
<App
1296+
todos={[
1297+
{ text: '1' },
1298+
{ text: '1A' },
1299+
{ text: '1B' },
1300+
{ text: '1C' },
1301+
{ text: '2' },
1302+
{ text: '2A' },
1303+
{ text: '2B' },
1304+
{ text: '2C' },
1305+
{ text: '3' }
1306+
]}
1307+
/>,
1308+
scratch
1309+
);
1310+
expect(scratch.querySelector('ul').textContent).to.equal('11A1B1C22A2B2C3');
1311+
1312+
// Collapse row 2: removes 2A, 2B, 2C
1313+
render(
1314+
<App
1315+
todos={[
1316+
{ text: '1' },
1317+
{ text: '1A' },
1318+
{ text: '1B' },
1319+
{ text: '1C' },
1320+
{ text: '2' },
1321+
{ text: '3' }
1322+
]}
1323+
/>,
1324+
scratch
1325+
);
1326+
expect(scratch.querySelector('ul').textContent).to.equal('11A1B1C23');
1327+
});
1328+
1329+
// https://github.com/preactjs/preact/issues/5065
1330+
it('should maintain correct DOM order expanding then collapsing with conditional Provider', () => {
1331+
const Ctx = createContext(null);
1332+
1333+
class Label extends Component {
1334+
render({ todo }) {
1335+
return <li key={todo.text}>{todo.text}</li>;
1336+
}
1337+
}
1338+
1339+
class App extends Component {
1340+
render() {
1341+
const { todos } = this.props;
1342+
return (
1343+
<ul>
1344+
{todos.map((todo, index) =>
1345+
todo.text === '1' || todo.text === '2' || todo.text === '3' ? (
1346+
<Ctx.Provider value={null}>
1347+
<Label todo={todo} index={index} />
1348+
</Ctx.Provider>
1349+
) : (
1350+
<Label todo={todo} index={index} />
1351+
)
1352+
)}
1353+
</ul>
1354+
);
1355+
}
1356+
}
1357+
1358+
// Initial: 3 rows collapsed
1359+
render(
1360+
<App todos={[{ text: '1' }, { text: '2' }, { text: '3' }]} />,
1361+
scratch
1362+
);
1363+
expect(scratch.querySelector('ul').textContent).to.equal('123');
1364+
1365+
// Expand row 1
1366+
render(
1367+
<App
1368+
todos={[
1369+
{ text: '1' },
1370+
{ text: '1A' },
1371+
{ text: '1B' },
1372+
{ text: '1C' },
1373+
{ text: '2' },
1374+
{ text: '3' }
1375+
]}
1376+
/>,
1377+
scratch
1378+
);
1379+
expect(scratch.querySelector('ul').textContent).to.equal('11A1B1C23');
1380+
1381+
// Collapse row 1
1382+
render(
1383+
<App todos={[{ text: '1' }, { text: '2' }, { text: '3' }]} />,
1384+
scratch
1385+
);
1386+
expect(scratch.querySelector('ul').textContent).to.equal('123');
1387+
});
11691388
});

0 commit comments

Comments
 (0)