From 787dacc70e32ce4d9eda8b36561592d32198bda0 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Thu, 23 Jun 2022 17:01:43 +0200 Subject: [PATCH 01/20] alternative use id --- hooks/src/index.js | 60 ++++++++++++++------- hooks/test/browser/useId.test.js | 91 ++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 20 deletions(-) create mode 100644 hooks/test/browser/useId.test.js diff --git a/hooks/src/index.js b/hooks/src/index.js index ffb7899822..e2b5a36658 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -26,15 +26,45 @@ let oldBeforeUnmount = options.unmount; const RAF_TIMEOUT = 100; let prevRaf; +let mask = 0; +let localIdCounter = 0; + options._diff = vnode => { + if (typeof vnode.type === 'function' && !vnode._mask) { + vnode._mask = + vnode._parent && vnode._parent._mask + ? vnode._parent && vnode._parent._mask + 1 + : mask++; + } currentComponent = null; if (oldBeforeDiff) oldBeforeDiff(vnode); }; +options.diffed = vnode => { + if (oldAfterDiff) oldAfterDiff(vnode); + + const c = vnode._component; + if (c && c.__hooks) { + if (c.__hooks._pendingEffects.length) afterPaint(afterPaintEffects.push(c)); + c.__hooks._list.forEach(hookItem => { + if (hookItem._pendingArgs) { + hookItem._args = hookItem._pendingArgs; + } + if (hookItem._pendingValue !== EMPTY) { + hookItem._value = hookItem._pendingValue; + } + hookItem._pendingArgs = undefined; + hookItem._pendingValue = EMPTY; + }); + } + previousComponent = currentComponent = null; +}; + options._render = vnode => { if (oldBeforeRender) oldBeforeRender(vnode); currentComponent = vnode._component; + localIdCounter = 0; currentIndex = 0; const hooks = currentComponent.__hooks; @@ -58,27 +88,8 @@ options._render = vnode => { previousComponent = currentComponent; }; -options.diffed = vnode => { - if (oldAfterDiff) oldAfterDiff(vnode); - - const c = vnode._component; - if (c && c.__hooks) { - if (c.__hooks._pendingEffects.length) afterPaint(afterPaintEffects.push(c)); - c.__hooks._list.forEach(hookItem => { - if (hookItem._pendingArgs) { - hookItem._args = hookItem._pendingArgs; - } - if (hookItem._pendingValue !== EMPTY) { - hookItem._value = hookItem._pendingValue; - } - hookItem._pendingArgs = undefined; - hookItem._pendingValue = EMPTY; - }); - } - previousComponent = currentComponent = null; -}; - options._commit = (vnode, commitQueue) => { + mask = 0; commitQueue.some(component => { try { component._renderCallbacks.forEach(invokeCleanup); @@ -366,6 +377,15 @@ export function useErrorBoundary(cb) { ]; } +export function useId() { + const state = getHookState(currentIndex++, 11); + if (!state._value) { + state._value = '_P' + currentComponent._vnode._mask + localIdCounter++; + } + + return state._value; +} + /** * After paint effects consumer. */ diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js new file mode 100644 index 0000000000..ba80642f08 --- /dev/null +++ b/hooks/test/browser/useId.test.js @@ -0,0 +1,91 @@ +import { createElement, render } from 'preact'; +import { setupScratch, teardown } from '../../../test/_util/helpers'; +import { useId } from 'preact/hooks'; + +/** @jsx createElement */ + +describe('useId', () => { + /** @type {HTMLDivElement} */ + let scratch; + + beforeEach(() => { + scratch = setupScratch(); + }); + + afterEach(() => { + teardown(scratch); + }); + + it('keeps the id consistent after an update', () => { + function Comp() { + const id = useId(); + return
; + } + + render(, scratch); + const id = scratch.firstChild.getAttribute('id'); + expect(scratch.firstChild.getAttribute('id')).to.equal(id); + + render(, scratch); + expect(scratch.firstChild.getAttribute('id')).to.equal(id); + }); + + it('ids are unique according to dom-depth', () => { + function Child() { + const id = useId(); + const spanId = useId(); + return ( +
+ h +
+ ); + } + + function Comp() { + const id = useId(); + return ( +
+ +
+ ); + } + + render(, scratch); + expect(scratch.innerHTML).to.equal( + '
h
' + ); + + render(, scratch); + expect(scratch.innerHTML).to.equal( + '
h
' + ); + }); + + it('ids are unique across siblings', () => { + function Child() { + const id = useId(); + return h; + } + + function Comp() { + const id = useId(); + return ( +
+ + + +
+ ); + } + + render(, scratch); + expect(scratch.innerHTML).to.equal( + '
hhh
' + ); + + render(, scratch); + expect(scratch.innerHTML).to.equal( + '
hhh
' + ); + }); +}); From da66eef173ac00e65544b8a0ae53a29803aa47f5 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 3 Jul 2022 10:00:34 +0200 Subject: [PATCH 02/20] reliable tagging --- hooks/src/index.js | 19 ++++++++++++------- hooks/test/browser/useId.test.js | 8 ++++---- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index e2b5a36658..e3ad1e1cf6 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -1,4 +1,4 @@ -import { options } from 'preact'; +import { Fragment, options } from 'preact'; /** @type {number} */ let currentIndex; @@ -26,15 +26,19 @@ let oldBeforeUnmount = options.unmount; const RAF_TIMEOUT = 100; let prevRaf; -let mask = 0; let localIdCounter = 0; options._diff = vnode => { if (typeof vnode.type === 'function' && !vnode._mask) { - vnode._mask = - vnode._parent && vnode._parent._mask - ? vnode._parent && vnode._parent._mask + 1 - : mask++; + const parentMask = + vnode._parent && vnode._parent._mask ? vnode._parent._mask : ''; + const position = + vnode._parent && vnode._parent._children + ? vnode._parent._children.indexOf(vnode) + : 0; + vnode._mask = parentMask + position; + } else { + vnode._mask = vnode._parent._mask; } currentComponent = null; if (oldBeforeDiff) oldBeforeDiff(vnode); @@ -380,7 +384,8 @@ export function useErrorBoundary(cb) { export function useId() { const state = getHookState(currentIndex++, 11); if (!state._value) { - state._value = '_P' + currentComponent._vnode._mask + localIdCounter++; + state._value = + '_P' + currentComponent._vnode._mask.toString(32) + localIdCounter++; } return state._value; diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index ba80642f08..3ca65b56ab 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -52,12 +52,12 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal( - '
h
' + '
h
' ); render(, scratch); expect(scratch.innerHTML).to.equal( - '
h
' + '
h
' ); }); @@ -80,12 +80,12 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal( - '
hhh
' + '
hhh
' ); render(, scratch); expect(scratch.innerHTML).to.equal( - '
hhh
' + '
hhh
' ); }); }); From c12615a44aba1c8029889d22ce87d660de1729fb Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 3 Jul 2022 10:03:39 +0200 Subject: [PATCH 03/20] skip fragments --- hooks/src/index.js | 7 +++++-- hooks/test/browser/useId.test.js | 8 ++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index e3ad1e1cf6..827fd00d3e 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -29,7 +29,10 @@ let prevRaf; let localIdCounter = 0; options._diff = vnode => { - if (typeof vnode.type === 'function' && !vnode._mask) { + if (vnode.type === Fragment) { + // Skip so we don't have to ensure wrapping fragments in RTS and prepass + vnode._mask = ''; + } else if (typeof vnode.type === 'function' && !vnode._mask) { const parentMask = vnode._parent && vnode._parent._mask ? vnode._parent._mask : ''; const position = @@ -385,7 +388,7 @@ export function useId() { const state = getHookState(currentIndex++, 11); if (!state._value) { state._value = - '_P' + currentComponent._vnode._mask.toString(32) + localIdCounter++; + 'P' + currentComponent._vnode._mask.toString(32) + localIdCounter++; } return state._value; diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index 3ca65b56ab..ea8846841a 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -52,12 +52,12 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal( - '
h
' + '
h
' ); render(, scratch); expect(scratch.innerHTML).to.equal( - '
h
' + '
h
' ); }); @@ -80,12 +80,12 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal( - '
hhh
' + '
hhh
' ); render(, scratch); expect(scratch.innerHTML).to.equal( - '
hhh
' + '
hhh
' ); }); }); From f410104a9f72dbad0c982117b47ef229e09bdb0d Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 3 Jul 2022 10:05:00 +0200 Subject: [PATCH 04/20] remove mask var --- hooks/src/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index 827fd00d3e..32b216af4d 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -96,7 +96,6 @@ options._render = vnode => { }; options._commit = (vnode, commitQueue) => { - mask = 0; commitQueue.some(component => { try { component._renderCallbacks.forEach(invokeCleanup); From 530af1dd44e2d29176c9f07ac0459ca1f0d038c3 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 3 Jul 2022 10:05:38 +0200 Subject: [PATCH 05/20] reduce diff --- hooks/src/index.js | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index 32b216af4d..3d8751eb28 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -47,26 +47,6 @@ options._diff = vnode => { if (oldBeforeDiff) oldBeforeDiff(vnode); }; -options.diffed = vnode => { - if (oldAfterDiff) oldAfterDiff(vnode); - - const c = vnode._component; - if (c && c.__hooks) { - if (c.__hooks._pendingEffects.length) afterPaint(afterPaintEffects.push(c)); - c.__hooks._list.forEach(hookItem => { - if (hookItem._pendingArgs) { - hookItem._args = hookItem._pendingArgs; - } - if (hookItem._pendingValue !== EMPTY) { - hookItem._value = hookItem._pendingValue; - } - hookItem._pendingArgs = undefined; - hookItem._pendingValue = EMPTY; - }); - } - previousComponent = currentComponent = null; -}; - options._render = vnode => { if (oldBeforeRender) oldBeforeRender(vnode); @@ -95,6 +75,26 @@ options._render = vnode => { previousComponent = currentComponent; }; +options.diffed = vnode => { + if (oldAfterDiff) oldAfterDiff(vnode); + + const c = vnode._component; + if (c && c.__hooks) { + if (c.__hooks._pendingEffects.length) afterPaint(afterPaintEffects.push(c)); + c.__hooks._list.forEach(hookItem => { + if (hookItem._pendingArgs) { + hookItem._args = hookItem._pendingArgs; + } + if (hookItem._pendingValue !== EMPTY) { + hookItem._value = hookItem._pendingValue; + } + hookItem._pendingArgs = undefined; + hookItem._pendingValue = EMPTY; + }); + } + previousComponent = currentComponent = null; +}; + options._commit = (vnode, commitQueue) => { commitQueue.some(component => { try { From 1858d396a68398a16faf18f942b91405b2e70de4 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 3 Jul 2022 10:07:25 +0200 Subject: [PATCH 06/20] types --- hooks/src/index.d.ts | 2 ++ hooks/src/internal.d.ts | 7 ++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/hooks/src/index.d.ts b/hooks/src/index.d.ts index 72322bd434..7b42d69fda 100644 --- a/hooks/src/index.d.ts +++ b/hooks/src/index.d.ts @@ -137,3 +137,5 @@ export function useDebugValue(value: T, formatter?: (value: T) => any): void; export function useErrorBoundary( callback?: (error: any, errorInfo: ErrorInfo) => Promise | void ): [any, () => void]; + +export function useId(): string; diff --git a/hooks/src/internal.d.ts b/hooks/src/internal.d.ts index 1bbfaaf093..58a2741384 100644 --- a/hooks/src/internal.d.ts +++ b/hooks/src/internal.d.ts @@ -1,7 +1,8 @@ import { Component as PreactComponent, PreactContext, - ErrorInfo + ErrorInfo, + VNode as PreactVNode } from '../../src/internal'; import { Reducer } from '.'; @@ -37,6 +38,10 @@ export interface Component extends PreactComponent { __hooks?: ComponentHooks; } +export interface VNode extends PreactVNode { + _mask?: string; +} + export type HookState = | EffectHookState | MemoHookState From 2516eb1c2096b470b3b87e4e6f192aa8806dcbbf Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 3 Jul 2022 10:19:52 +0200 Subject: [PATCH 07/20] use hash func" --- hooks/src/index.js | 12 ++++++++++-- hooks/test/browser/useId.test.js | 8 ++++---- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index 3d8751eb28..d7a1818682 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -383,11 +383,19 @@ export function useErrorBoundary(cb) { ]; } +function hash(s) { + let h = 0, + i = s.length; + while (i > 0) { + h = ((h << 5) - h + s.charCodeAt(--i)) | 0; + } + return h; +} + export function useId() { const state = getHookState(currentIndex++, 11); if (!state._value) { - state._value = - 'P' + currentComponent._vnode._mask.toString(32) + localIdCounter++; + state._value = 'P' + hash(currentComponent._vnode._mask) + localIdCounter++; } return state._value; diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index ea8846841a..841012ce51 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -52,12 +52,12 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal( - '
h
' + '
h
' ); render(, scratch); expect(scratch.innerHTML).to.equal( - '
h
' + '
h
' ); }); @@ -80,12 +80,12 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal( - '
hhh
' + '
hhh
' ); render(, scratch); expect(scratch.innerHTML).to.equal( - '
hhh
' + '
hhh
' ); }); }); From 1337fdf363dc8232390fcce6334fd3ca1fc8b730 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 3 Jul 2022 11:30:05 +0200 Subject: [PATCH 08/20] less bytes --- hooks/src/index.js | 12 +++++------- hooks/test/browser/useId.test.js | 8 ++++---- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index d7a1818682..691d515425 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -383,13 +383,11 @@ export function useErrorBoundary(cb) { ]; } -function hash(s) { - let h = 0, - i = s.length; - while (i > 0) { - h = ((h << 5) - h + s.charCodeAt(--i)) | 0; - } - return h; +function hash(str) { + let i = 0, + out = 11; + while (i < str.length) out = (101 * out + str.charCodeAt(i++)) >>> 0; + return out; } export function useId() { diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index 841012ce51..4802f289a7 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -52,12 +52,12 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal( - '
h
' + '
h
' ); render(, scratch); expect(scratch.innerHTML).to.equal( - '
h
' + '
h
' ); }); @@ -80,12 +80,12 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal( - '
hhh
' + '
hhh
' ); render(, scratch); expect(scratch.innerHTML).to.equal( - '
hhh
' + '
hhh
' ); }); }); From 07074fb816e0c6c3e1dbd1121b77db2312dce47b Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 3 Jul 2022 11:32:07 +0200 Subject: [PATCH 09/20] fix inheritance bug --- hooks/src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index 691d515425..685095ca57 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -40,7 +40,7 @@ options._diff = vnode => { ? vnode._parent._children.indexOf(vnode) : 0; vnode._mask = parentMask + position; - } else { + } else if (!vnode._mask) { vnode._mask = vnode._parent._mask; } currentComponent = null; From 24733ea5cbf668575960b7851c8f9d03e9b62247 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 3 Jul 2022 12:20:16 +0200 Subject: [PATCH 10/20] optimize --- hooks/src/index.js | 5 +---- hooks/test/browser/useId.test.js | 8 ++++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index 685095ca57..d78ef36736 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -26,8 +26,6 @@ let oldBeforeUnmount = options.unmount; const RAF_TIMEOUT = 100; let prevRaf; -let localIdCounter = 0; - options._diff = vnode => { if (vnode.type === Fragment) { // Skip so we don't have to ensure wrapping fragments in RTS and prepass @@ -51,7 +49,6 @@ options._render = vnode => { if (oldBeforeRender) oldBeforeRender(vnode); currentComponent = vnode._component; - localIdCounter = 0; currentIndex = 0; const hooks = currentComponent.__hooks; @@ -393,7 +390,7 @@ function hash(str) { export function useId() { const state = getHookState(currentIndex++, 11); if (!state._value) { - state._value = 'P' + hash(currentComponent._vnode._mask) + localIdCounter++; + state._value = 'P' + hash(currentComponent._vnode._mask + currentIndex); } return state._value; diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index 4802f289a7..fbcd380191 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -52,12 +52,12 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal( - '
h
' + '
h
' ); render(, scratch); expect(scratch.innerHTML).to.equal( - '
h
' + '
h
' ); }); @@ -80,12 +80,12 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal( - '
hhh
' + '
hhh
' ); render(, scratch); expect(scratch.innerHTML).to.equal( - '
hhh
' + '
hhh
' ); }); }); From dd5df426c055c672ff80e4d4c2e6975a4e4a8d35 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Thu, 18 Aug 2022 13:10:03 +0200 Subject: [PATCH 11/20] tweak logic --- hooks/src/index.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index d78ef36736..f5eac96402 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -27,19 +27,16 @@ const RAF_TIMEOUT = 100; let prevRaf; options._diff = vnode => { - if (vnode.type === Fragment) { - // Skip so we don't have to ensure wrapping fragments in RTS and prepass - vnode._mask = ''; - } else if (typeof vnode.type === 'function' && !vnode._mask) { + if (typeof vnode.type === "function" && !vnode._mask && vnode.type !== Fragment) { const parentMask = - vnode._parent && vnode._parent._mask ? vnode._parent._mask : ''; + vnode[PARENT] && vnode[PARENT]._mask ? vnode[PARENT]._mask : ""; const position = - vnode._parent && vnode._parent._children - ? vnode._parent._children.indexOf(vnode) - : 0; + vnode[PARENT] && vnode[PARENT][CHILDREN] + ? vnode[PARENT][CHILDREN].indexOf(vnode) + : 0; vnode._mask = parentMask + position; } else if (!vnode._mask) { - vnode._mask = vnode._parent._mask; + vnode._mask = vnode[PARENT] ? vnode[PARENT]._mask : ""; } currentComponent = null; if (oldBeforeDiff) oldBeforeDiff(vnode); From 6e95370c21f8202ef8f1fc8f7c2ac82f39834a55 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Thu, 18 Aug 2022 13:18:39 +0200 Subject: [PATCH 12/20] fix copy pate --- hooks/src/index.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index f5eac96402..ca53247934 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -27,16 +27,20 @@ const RAF_TIMEOUT = 100; let prevRaf; options._diff = vnode => { - if (typeof vnode.type === "function" && !vnode._mask && vnode.type !== Fragment) { + if ( + typeof vnode.type === 'function' && + !vnode._mask && + vnode.type !== Fragment + ) { const parentMask = - vnode[PARENT] && vnode[PARENT]._mask ? vnode[PARENT]._mask : ""; + vnode._parent && vnode._parent._mask ? vnode._parent._mask : ''; const position = - vnode[PARENT] && vnode[PARENT][CHILDREN] - ? vnode[PARENT][CHILDREN].indexOf(vnode) - : 0; + vnode._parent && vnode._parent._children + ? vnode._parent._children.indexOf(vnode) + : 0; vnode._mask = parentMask + position; } else if (!vnode._mask) { - vnode._mask = vnode[PARENT] ? vnode[PARENT]._mask : ""; + vnode._mask = vnode._parent ? vnode._parent._mask : ''; } currentComponent = null; if (oldBeforeDiff) oldBeforeDiff(vnode); From 85ede7d42cdea65f998b9a8375d54b796b6bd3b1 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Thu, 1 Sep 2022 19:38:17 +0200 Subject: [PATCH 13/20] less bytes --- hooks/src/index.js | 51 ++++++++++++++++++-------------- hooks/test/browser/useId.test.js | 8 ++--- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index ca53247934..5dbcc0bac9 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -26,23 +26,31 @@ let oldBeforeUnmount = options.unmount; const RAF_TIMEOUT = 100; let prevRaf; +// the current ID prefix +let mask = ''; + +// tracks component depth (only for the current render, so may be subtree depth) +let depth = 0; + options._diff = vnode => { - if ( - typeof vnode.type === 'function' && - !vnode._mask && - vnode.type !== Fragment - ) { - const parentMask = - vnode._parent && vnode._parent._mask ? vnode._parent._mask : ''; - const position = - vnode._parent && vnode._parent._children - ? vnode._parent._children.indexOf(vnode) - : 0; - vnode._mask = parentMask + position; - } else if (!vnode._mask) { - vnode._mask = vnode._parent ? vnode._parent._mask : ''; + if (typeof vnode.type === 'function' && vnode.type !== Fragment) { + if (vnode._mask) { + // already visited, pick up where we left off: (fixes subtree updates) + mask = vnode._mask; + } else if (depth === 0) { + // this is the first component rendered and hasn't been seen, it's a root: + vnode._mask = mask = ''; + } else { + // replace mask[depth] with the next character: (basically *mask[depth-1]++) + const offset = Number(mask[depth - 1]) + 1; + vnode._mask = mask = mask.slice(0, depth - 1) + offset; + } + // we're about to render the children of this component, push a new level onto the mask: + depth++; + mask += '0'; // next level starts off at the base character (code 65) again } currentComponent = null; + if (oldBeforeDiff) oldBeforeDiff(vnode); }; @@ -76,6 +84,12 @@ options._render = vnode => { options.diffed = vnode => { if (oldAfterDiff) oldAfterDiff(vnode); + if (typeof vnode.type === 'function' && vnode.type !== Fragment) { + // jump back up the stack, remove the last char from the mask: + depth--; + mask = mask.slice(0, -1); + } + const c = vnode._component; if (c && c.__hooks) { if (c.__hooks._pendingEffects.length) afterPaint(afterPaintEffects.push(c)); @@ -381,17 +395,10 @@ export function useErrorBoundary(cb) { ]; } -function hash(str) { - let i = 0, - out = 11; - while (i < str.length) out = (101 * out + str.charCodeAt(i++)) >>> 0; - return out; -} - export function useId() { const state = getHookState(currentIndex++, 11); if (!state._value) { - state._value = 'P' + hash(currentComponent._vnode._mask + currentIndex); + state._value = 'P' + currentComponent._vnode._mask + '-' + currentIndex; } return state._value; diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index fbcd380191..52019c5c70 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -52,12 +52,12 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal( - '
h
' + '
h
' ); render(, scratch); expect(scratch.innerHTML).to.equal( - '
h
' + '
h
' ); }); @@ -80,12 +80,12 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal( - '
hhh
' + '
hhh
' ); render(, scratch); expect(scratch.innerHTML).to.equal( - '
hhh
' + '
hhh
' ); }); }); From 7e095a518cbf42b81105b0dd0ef65e80fd7caae3 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Thu, 1 Sep 2022 19:38:44 +0200 Subject: [PATCH 14/20] add compat --- compat/src/index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/compat/src/index.js b/compat/src/index.js index 49c67a7aa8..6b9988951c 100644 --- a/compat/src/index.js +++ b/compat/src/index.js @@ -9,6 +9,7 @@ import { } from 'preact'; import { useState, + useId, useReducer, useEffect, useLayoutEffect, @@ -201,6 +202,7 @@ export { // React copies the named exports to the default one. export default { useState, + useId, useReducer, useEffect, useLayoutEffect, From b9b276f6bb2d7d49a5145171c1bd2e06078d5b7c Mon Sep 17 00:00:00 2001 From: jdecroock Date: Thu, 1 Sep 2022 19:39:17 +0200 Subject: [PATCH 15/20] types --- compat/src/index.d.ts | 1 + hooks/src/index.d.ts | 2 +- hooks/src/index.js | 7 +++---- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compat/src/index.d.ts b/compat/src/index.d.ts index 45b7a177de..eaf108fee1 100644 --- a/compat/src/index.d.ts +++ b/compat/src/index.d.ts @@ -24,6 +24,7 @@ declare namespace React { export import useDebugValue = _hooks.useDebugValue; export import useEffect = _hooks.useEffect; export import useImperativeHandle = _hooks.useImperativeHandle; + export import useId = _hooks.useId; export import useLayoutEffect = _hooks.useLayoutEffect; export import useMemo = _hooks.useMemo; export import useReducer = _hooks.useReducer; diff --git a/hooks/src/index.d.ts b/hooks/src/index.d.ts index 7b42d69fda..dfec612eaa 100644 --- a/hooks/src/index.d.ts +++ b/hooks/src/index.d.ts @@ -138,4 +138,4 @@ export function useErrorBoundary( callback?: (error: any, errorInfo: ErrorInfo) => Promise | void ): [any, () => void]; -export function useId(): string; +export function useId(): string; diff --git a/hooks/src/index.js b/hooks/src/index.js index 5dbcc0bac9..cff822c062 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -41,13 +41,13 @@ options._diff = vnode => { // this is the first component rendered and hasn't been seen, it's a root: vnode._mask = mask = ''; } else { - // replace mask[depth] with the next character: (basically *mask[depth-1]++) + // replace mask[depth] with the next: (basically *mask[depth-1]++) const offset = Number(mask[depth - 1]) + 1; vnode._mask = mask = mask.slice(0, depth - 1) + offset; } - // we're about to render the children of this component, push a new level onto the mask: + depth++; - mask += '0'; // next level starts off at the base character (code 65) again + mask += '0'; } currentComponent = null; @@ -85,7 +85,6 @@ options.diffed = vnode => { if (oldAfterDiff) oldAfterDiff(vnode); if (typeof vnode.type === 'function' && vnode.type !== Fragment) { - // jump back up the stack, remove the last char from the mask: depth--; mask = mask.slice(0, -1); } From 1202b8310c237fa7aee7a680800d6561980a3525 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Thu, 1 Sep 2022 19:42:36 +0200 Subject: [PATCH 16/20] add consideration --- hooks/src/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/hooks/src/index.js b/hooks/src/index.js index cff822c062..1b7f89da5c 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -397,6 +397,7 @@ export function useErrorBoundary(cb) { export function useId() { const state = getHookState(currentIndex++, 11); if (!state._value) { + // TODO: consider Number(currentComponent._vnode._mask + currentIndex).toString(36) state._value = 'P' + currentComponent._vnode._mask + '-' + currentIndex; } From 47f03e8cdcfc0bd9aeccf15da79c8aba92a00149 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Fri, 2 Sep 2022 10:27:18 +0200 Subject: [PATCH 17/20] add useId test highlighting subsequent render dangers --- hooks/test/browser/useId.test.js | 45 ++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index 52019c5c70..cc90f76853 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -1,15 +1,17 @@ import { createElement, render } from 'preact'; +import { useId, useState } from 'preact/hooks'; +import { setupRerender } from 'preact/test-utils'; import { setupScratch, teardown } from '../../../test/_util/helpers'; -import { useId } from 'preact/hooks'; /** @jsx createElement */ describe('useId', () => { /** @type {HTMLDivElement} */ - let scratch; + let scratch, rerender; beforeEach(() => { scratch = setupScratch(); + rerender = setupRerender(); }); afterEach(() => { @@ -88,4 +90,43 @@ describe('useId', () => { '
hhh
' ); }); + + it('correctly handles new elements', () => { + let set; + function Child() { + const id = useId(); + return h; + } + + function Stateful() { + const [state, setState] = useState(false); + set = setState; + return ( +
+ + {state && } +
+ ); + } + + function Comp() { + const id = useId(); + return ( +
+ +
+ ); + } + + render(, scratch); + expect(scratch.innerHTML).to.equal( + '
h
' + ); + + set(true); + rerender(); + expect(scratch.innerHTML).to.equal( + '
hh
' + ); + }); }); From 1f4dedad8b92579ab036cf6dfaed217972c59f38 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Fri, 2 Sep 2022 10:32:57 +0200 Subject: [PATCH 18/20] fix client side renders --- hooks/src/index.js | 4 ++++ hooks/test/browser/useId.test.js | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index 1b7f89da5c..77fab8b27d 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -37,18 +37,22 @@ options._diff = vnode => { if (vnode._mask) { // already visited, pick up where we left off: (fixes subtree updates) mask = vnode._mask; + depth = vnode._maskDepth; } else if (depth === 0) { // this is the first component rendered and hasn't been seen, it's a root: vnode._mask = mask = ''; + vnode._maskDepth = depth; } else { // replace mask[depth] with the next: (basically *mask[depth-1]++) const offset = Number(mask[depth - 1]) + 1; vnode._mask = mask = mask.slice(0, depth - 1) + offset; + vnode._maskDepth = depth; } depth++; mask += '0'; } + currentComponent = null; if (oldBeforeDiff) oldBeforeDiff(vnode); diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index cc90f76853..60d0cb0db3 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -126,7 +126,7 @@ describe('useId', () => { set(true); rerender(); expect(scratch.innerHTML).to.equal( - '
hh
' + '
hh
' ); }); }); From cf952ad7b38bc1b22018cb05e162efb918198658 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Fri, 2 Sep 2022 16:39:16 +0200 Subject: [PATCH 19/20] revert to prior implementation --- hooks/src/index.js | 54 +++++++++++++------------------- hooks/test/browser/useId.test.js | 12 +++---- 2 files changed, 27 insertions(+), 39 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index 77fab8b27d..dbe21d8070 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -26,35 +26,21 @@ let oldBeforeUnmount = options.unmount; const RAF_TIMEOUT = 100; let prevRaf; -// the current ID prefix -let mask = ''; - -// tracks component depth (only for the current render, so may be subtree depth) -let depth = 0; - options._diff = vnode => { - if (typeof vnode.type === 'function' && vnode.type !== Fragment) { - if (vnode._mask) { - // already visited, pick up where we left off: (fixes subtree updates) - mask = vnode._mask; - depth = vnode._maskDepth; - } else if (depth === 0) { - // this is the first component rendered and hasn't been seen, it's a root: - vnode._mask = mask = ''; - vnode._maskDepth = depth; - } else { - // replace mask[depth] with the next: (basically *mask[depth-1]++) - const offset = Number(mask[depth - 1]) + 1; - vnode._mask = mask = mask.slice(0, depth - 1) + offset; - vnode._maskDepth = depth; - } - - depth++; - mask += '0'; + if (vnode.type === Fragment) { + // Skip so we don't have to ensure wrapping fragments in RTS and prepass + vnode._mask = ''; + } else if (typeof vnode.type === 'function' && !vnode._mask) { + vnode._mask = + (vnode._parent && vnode._parent._mask ? vnode._parent._mask : '') + + (vnode._parent && vnode._parent._children + ? vnode._parent._children.indexOf(vnode) + : 0); + } else { + vnode._mask = vnode._parent._mask; } currentComponent = null; - if (oldBeforeDiff) oldBeforeDiff(vnode); }; @@ -88,11 +74,6 @@ options._render = vnode => { options.diffed = vnode => { if (oldAfterDiff) oldAfterDiff(vnode); - if (typeof vnode.type === 'function' && vnode.type !== Fragment) { - depth--; - mask = mask.slice(0, -1); - } - const c = vnode._component; if (c && c.__hooks) { if (c.__hooks._pendingEffects.length) afterPaint(afterPaintEffects.push(c)); @@ -398,16 +379,23 @@ export function useErrorBoundary(cb) { ]; } +function hash(s) { + let h = 0, + i = s.length; + while (i > 0) { + h = ((h << 5) - h + s.charCodeAt(--i)) | 0; + } + return h; +} + export function useId() { const state = getHookState(currentIndex++, 11); if (!state._value) { - // TODO: consider Number(currentComponent._vnode._mask + currentIndex).toString(36) - state._value = 'P' + currentComponent._vnode._mask + '-' + currentIndex; + state._value = 'P' + hash(currentComponent._vnode._mask) + currentIndex; } return state._value; } - /** * After paint effects consumer. */ diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index 60d0cb0db3..5c0dad0d53 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -54,12 +54,12 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal( - '
h
' + '
h
' ); render(, scratch); expect(scratch.innerHTML).to.equal( - '
h
' + '
h
' ); }); @@ -82,12 +82,12 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal( - '
hhh
' + '
hhh
' ); render(, scratch); expect(scratch.innerHTML).to.equal( - '
hhh
' + '
hhh
' ); }); @@ -120,13 +120,13 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal( - '
h
' + '
h
' ); set(true); rerender(); expect(scratch.innerHTML).to.equal( - '
hh
' + '
hh
' ); }); }); From 1aad6d8066c5c9c0bcab5d3c8c20c1bf2eaf8cdc Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sat, 3 Sep 2022 09:19:50 +0200 Subject: [PATCH 20/20] optim --- hooks/src/index.js | 14 ++++++++------ hooks/test/browser/useId.test.js | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index dbe21d8070..40e1366cb0 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -27,17 +27,19 @@ const RAF_TIMEOUT = 100; let prevRaf; options._diff = vnode => { - if (vnode.type === Fragment) { - // Skip so we don't have to ensure wrapping fragments in RTS and prepass - vnode._mask = ''; - } else if (typeof vnode.type === 'function' && !vnode._mask) { + if ( + typeof vnode.type === 'function' && + !vnode._mask && + vnode.type !== Fragment + ) { vnode._mask = (vnode._parent && vnode._parent._mask ? vnode._parent._mask : '') + (vnode._parent && vnode._parent._children ? vnode._parent._children.indexOf(vnode) : 0); - } else { - vnode._mask = vnode._parent._mask; + } else if (!vnode._mask) { + vnode._mask = + vnode._parent && vnode._parent._mask ? vnode._parent._mask : ''; } currentComponent = null; diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index 5c0dad0d53..53f3b893af 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -126,7 +126,7 @@ describe('useId', () => { set(true); rerender(); expect(scratch.innerHTML).to.equal( - '
hh
' + '
hh
' ); }); });