Skip to content

Commit

Permalink
Fix useId not unique on shared component parent + DOM
Browse files Browse the repository at this point in the history
  • Loading branch information
marvinhagemeister committed Oct 21, 2022
1 parent 9d4b2dc commit 4092391
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 31 deletions.
42 changes: 24 additions & 18 deletions hooks/src/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { options } from 'preact';
import { Fragment, options } from 'preact';

/** @type {number} */
let currentIndex;
Expand Down Expand Up @@ -27,22 +27,6 @@ const RAF_TIMEOUT = 100;
let prevRaf;

options._diff = vnode => {
if (
typeof vnode.type === 'function' &&
!vnode._mask &&
// Ignore root Fragment node
vnode._parent !== null
) {
vnode._mask =
(vnode._parent && vnode._parent._mask ? vnode._parent._mask : '') +
(vnode._parent && vnode._parent._children
? vnode._parent._children.indexOf(vnode)
: 0);
} else if (!vnode._mask) {
vnode._mask =
vnode._parent && vnode._parent._mask ? vnode._parent._mask : '';
}

currentComponent = null;
if (oldBeforeDiff) oldBeforeDiff(vnode);
};
Expand Down Expand Up @@ -395,7 +379,29 @@ function hash(s) {
export function useId() {
const state = getHookState(currentIndex++, 11);
if (!state._value) {
state._value = 'P' + hash(currentComponent._vnode._mask) + currentIndex;
// Traverse the tree upwards and count the components until we reach
// the root node.
let parent = currentComponent._vnode._parent;
let root = currentComponent._vnode;
let i = 0;
while (parent !== null) {
if (parent.type !== Fragment && typeof parent.type === 'function') {
i++;
}

root = parent;
parent = parent._parent;
}

// Attach the id usage array to the root node and resize it to fit
let ids = root._ids || (root._ids = [0]);
while (ids.length < i) {
ids.push(0);
}

// Increase the current component depth pointer
ids[i - 1]++;
state._value = 'P' + hash(ids.join('')) + currentIndex;
}

return state._value;
Expand Down
4 changes: 0 additions & 4 deletions hooks/src/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ export interface Component extends PreactComponent<any, any> {
__hooks?: ComponentHooks;
}

export interface VNode extends PreactVNode {
_mask?: string;
}

export type HookState =
| EffectHookState
| MemoHookState
Expand Down
40 changes: 33 additions & 7 deletions hooks/test/browser/useId.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ describe('useId', () => {

render(<Comp />, scratch);
expect(scratch.innerHTML).to.equal(
'<div id="P481"><div id="P15361"><span id="P15362">h</span></div></div>'
'<div id="P481"><div id="P491"><span id="P502">h</span></div></div>'
);

render(<Comp />, scratch);
expect(scratch.innerHTML).to.equal(
'<div id="P481"><div id="P15361"><span id="P15362">h</span></div></div>'
'<div id="P481"><div id="P491"><span id="P502">h</span></div></div>'
);
});

Expand All @@ -83,12 +83,12 @@ describe('useId', () => {

render(<Comp />, scratch);
expect(scratch.innerHTML).to.equal(
'<div id="P481"><span id="P15361">h</span><span id="P15671">h</span><span id="P15981">h</span></div>'
'<div id="P481"><span id="P491">h</span><span id="P501">h</span><span id="P511">h</span></div>'
);

render(<Comp />, scratch);
expect(scratch.innerHTML).to.equal(
'<div id="P481"><span id="P15361">h</span><span id="P15671">h</span><span id="P15981">h</span></div>'
'<div id="P481"><span id="P491">h</span><span id="P501">h</span><span id="P511">h</span></div>'
);
});

Expand Down Expand Up @@ -121,13 +121,13 @@ describe('useId', () => {

render(<Comp />, scratch);
expect(scratch.innerHTML).to.equal(
'<div id="P481"><div><span id="P476641">h</span></div></div>'
'<div id="P481"><div><span id="P15671">h</span></div></div>'
);

set(true);
rerender();
expect(scratch.innerHTML).to.equal(
'<div id="P481"><div><span id="P476641">h</span><span id="P486251">h</span></div></div>'
'<div id="P481"><div><span id="P15671">h</span><span id="P15981">h</span></div></div>'
);
});

Expand Down Expand Up @@ -350,6 +350,32 @@ describe('useId', () => {
}

render(<App />, scratch);
expect(scratch.innerHTML).to.equal('<div><p>P481</p><p>P476951</p></div>');
expect(scratch.innerHTML).to.equal('<div><p>P481</p><p>P15671</p></div>');
});

it('should skip over HTML', () => {
const ids = [];

function Foo() {
const id = useId();
ids.push(id);
return <p>{id}</p>;
}

function App() {
return (
<div>
<span>
<Foo />
</span>
<span>
<Foo />
</span>
</div>
);
}

render(<App />, scratch);
expect(ids).to.deep.equal(['P491', 'P501']);
});
});
3 changes: 1 addition & 2 deletions mangle.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
"$_depth": "__b",
"$_nextDom": "__d",
"$_dirty": "__d",
"$_mask": "__m",
"$_detachOnNextRender": "__b",
"$_force": "__e",
"$_nextState": "__s",
Expand Down Expand Up @@ -81,4 +80,4 @@
"$_isSuspended": "__i"
}
}
}
}

0 comments on commit 4092391

Please sign in to comment.