From f4512a80930c574ccc9f7fc40045d508eadd86f6 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 9 Jan 2022 12:00:14 +0100 Subject: [PATCH 01/16] initial implementation for the useId hook --- hooks/src/index.d.ts | 2 ++ hooks/src/index.js | 14 ++++++++ hooks/test/browser/useId.test.js | 55 ++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+) create mode 100644 hooks/test/browser/useId.test.js diff --git a/hooks/src/index.d.ts b/hooks/src/index.d.ts index a33efeafec..9e8a2c0ab8 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) => Promise | void ): [any, () => void]; + +export function useId(): string; diff --git a/hooks/src/index.js b/hooks/src/index.js index 1729a0a630..bba945dd9c 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -11,6 +11,9 @@ let currentInternal; /** @type {number} */ let currentHook = 0; +/** @type {number} */ +let currentIdCounter; + /** @type {Array} */ let afterPaintEffects = []; @@ -32,6 +35,7 @@ options._render = internal => { if (oldBeforeRender) oldBeforeRender(internal); currentInternal = internal; + currentIdCounter = 0; currentIndex = 0; if (currentInternal.data && currentInternal.data.__hooks) { @@ -307,6 +311,16 @@ export function useErrorBoundary(cb) { ]; } + +export function useId() { + const state = getHookState(currentIndex++, 11); + if (!state._id) { + currentIdCounter++; + state._id = '' + currentInternal._depth + currentIdCounter + } + return state._id; +} + /** * 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..cfb2d1b2dd --- /dev/null +++ b/hooks/test/browser/useId.test.js @@ -0,0 +1,55 @@ +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 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
') + }); +}); From a39127a6f35825d89790bb88d461a2678c61f286 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 9 Jan 2022 12:01:28 +0100 Subject: [PATCH 02/16] ensure uniqueness --- hooks/src/index.js | 2 +- hooks/test/browser/useId.test.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index bba945dd9c..eb7fc84c4d 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -316,7 +316,7 @@ export function useId() { const state = getHookState(currentIndex++, 11); if (!state._id) { currentIdCounter++; - state._id = '' + currentInternal._depth + currentIdCounter + state._id = '' + currentInternal._vnodeId + currentInternal._depth + currentIdCounter } return state._id; } diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index cfb2d1b2dd..f05355dbc0 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -47,9 +47,9 @@ describe('useId', () => { } render(, scratch); - expect(scratch.innerHTML).to.equal('
h
') + expect(scratch.innerHTML).to.equal('
h
') render(, scratch); - expect(scratch.innerHTML).to.equal('
h
') + expect(scratch.innerHTML).to.equal('
h
') }); }); From e815f7d55a97cde0f2bbc7db4469e4cd1ac03915 Mon Sep 17 00:00:00 2001 From: JoviDeCroock Date: Mon, 10 Jan 2022 15:56:10 +0100 Subject: [PATCH 03/16] try rootId approach --- hooks/src/index.js | 2 +- mangle.json | 1 + src/create-root.js | 4 +++- src/internal.d.ts | 6 +++++- src/options.js | 2 +- src/tree.js | 7 +++++-- 6 files changed, 16 insertions(+), 6 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index eb7fc84c4d..3310b13c93 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -316,7 +316,7 @@ export function useId() { const state = getHookState(currentIndex++, 11); if (!state._id) { currentIdCounter++; - state._id = '' + currentInternal._vnodeId + currentInternal._depth + currentIdCounter + state._id = '' + currentInternal._rootId + currentInternal._domDepth + currentIdCounter } return state._id; } diff --git a/mangle.json b/mangle.json index 14b3d639e2..fad2859ed5 100644 --- a/mangle.json +++ b/mangle.json @@ -44,6 +44,7 @@ "$_args": "__H", "$_factory": "__h", "$_depth": "__b", + "$_domDepth": "__dd", "$_detachOnNextRender": "__b", "$_nextState": "__s", "$_commitCallbacks": "__h", diff --git a/src/create-root.js b/src/create-root.js index 98dfefaccd..89f9473da8 100644 --- a/src/create-root.js +++ b/src/create-root.js @@ -11,6 +11,8 @@ import { mount } from './diff/mount'; import { patch } from './diff/patch'; import { createInternal } from './tree'; +let rootId = 0; + /** * * @param {import('./internal').PreactElement} parentDom The DOM element to @@ -36,7 +38,7 @@ export function createRoot(parentDom) { if (rootInternal) { patch(parentDom, vnode, rootInternal, commitQueue); } else { - rootInternal = createInternal(vnode); + rootInternal = createInternal(vnode, undefined, rootId); // Store the VDOM tree root on the DOM element in a (minified) property: parentDom._children = rootInternal; diff --git a/src/internal.d.ts b/src/internal.d.ts index f6485f7097..88fcf39731 100644 --- a/src/internal.d.ts +++ b/src/internal.d.ts @@ -158,8 +158,12 @@ export interface Internal

{ _component: Component | null; /** most recent context object passed down to this Internal from its parent */ _context: any; - /** This Internal's distance from the tree root */ + /** This Internal's distance from the tree root counted in vdom-nodes */ _depth: number | null; + /** This Internal's distance from the tree root counted in dom-nodes */ + _domDepth: number | null; + /** The id of the root */ + _rootId: number | null; /** Callbacks to invoke when this internal commits */ _commitCallbacks: Array<() => void>; } diff --git a/src/options.js b/src/options.js index 174f322705..8c9901a88a 100644 --- a/src/options.js +++ b/src/options.js @@ -10,7 +10,7 @@ import { _catchError } from './diff/catch-error'; * @type {import('./internal').Options} */ const options = { - _catchError + _catchError, }; export default options; diff --git a/src/tree.js b/src/tree.js index a302d1d139..66c90992aa 100644 --- a/src/tree.js +++ b/src/tree.js @@ -17,9 +17,10 @@ import { enqueueRender } from './component'; * Create an internal tree node * @param {import('./internal').VNode | string} vnode * @param {import('./internal').Internal} [parentInternal] + * @param {number} [rootId] * @returns {import('./internal').Internal} */ -export function createInternal(vnode, parentInternal) { +export function createInternal(vnode, parentInternal, rootId) { let type = null, props, key, @@ -93,7 +94,9 @@ export function createInternal(vnode, parentInternal) { _dom: null, _component: null, _context: null, - _depth: parentInternal ? parentInternal._depth + 1 : 0 + _depth: parentInternal ? parentInternal._depth + 1 : 0, + _rootId: rootId || parentInternal._rootId, + _domDepth: typeof type == 'function' ? (parentInternal._domDepth || 0) : (parentInternal._domDepth || 0) + 1 }; if (options._internal) options._internal(internal, vnode); From 420019e66cdda1db4eefcb11ebd478892c7294c0 Mon Sep 17 00:00:00 2001 From: JoviDeCroock Date: Mon, 10 Jan 2022 15:56:56 +0100 Subject: [PATCH 04/16] add mangle --- mangle.json | 1 + 1 file changed, 1 insertion(+) diff --git a/mangle.json b/mangle.json index fad2859ed5..b223bfb1a0 100644 --- a/mangle.json +++ b/mangle.json @@ -66,6 +66,7 @@ "$_originalParentDom": "__O", "$_prevState": "__u", "$_root": "__", + "$_rootId": "__ri", "$_diff": "__b", "$_commit": "__c", "$_addHookName": "__a", From 0a593fc48994c6a272c3b295c99bca61e85fd25e Mon Sep 17 00:00:00 2001 From: JoviDeCroock Date: Mon, 10 Jan 2022 16:00:20 +0100 Subject: [PATCH 05/16] fix mistake in tree creation --- src/tree.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tree.js b/src/tree.js index 66c90992aa..8cf98409b0 100644 --- a/src/tree.js +++ b/src/tree.js @@ -79,6 +79,7 @@ export function createInternal(vnode, parentInternal, rootId) { } } + const parentDomDepth = parentInternal ? parentInternal._domDepth : 0; /** @type {import('./internal').Internal} */ const internal = { type, @@ -95,8 +96,8 @@ export function createInternal(vnode, parentInternal, rootId) { _component: null, _context: null, _depth: parentInternal ? parentInternal._depth + 1 : 0, - _rootId: rootId || parentInternal._rootId, - _domDepth: typeof type == 'function' ? (parentInternal._domDepth || 0) : (parentInternal._domDepth || 0) + 1 + _rootId: rootId || (parentInternal && parentInternal._rootId || 0), + _domDepth: typeof type == 'function' ? parentDomDepth : parentDomDepth + 1 }; if (options._internal) options._internal(internal, vnode); From 2bd99b8a595b29c28ccbc776773dd3cb562dcec0 Mon Sep 17 00:00:00 2001 From: JoviDeCroock Date: Mon, 10 Jan 2022 16:02:58 +0100 Subject: [PATCH 06/16] fix test --- hooks/test/browser/useId.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index f05355dbc0..355f69dad5 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -30,7 +30,7 @@ describe('useId', () => { expect(scratch.firstChild.getAttribute('id')).to.equal(id) }); - it('ids are unique according to depth', () => { + it('ids are unique according to dom-depth', () => { function Child() { const id = useId() const spanId = useId() @@ -47,9 +47,9 @@ describe('useId', () => { } render(, scratch); - expect(scratch.innerHTML).to.equal('

h
') + expect(scratch.innerHTML).to.equal('
h
') render(, scratch); - expect(scratch.innerHTML).to.equal('
h
') + expect(scratch.innerHTML).to.equal('
h
') }); }); From 00203de9eac4415a196a37d561384d197248943f Mon Sep 17 00:00:00 2001 From: JoviDeCroock Date: Mon, 10 Jan 2022 16:23:17 +0100 Subject: [PATCH 07/16] add failing test --- hooks/src/index.js | 2 +- hooks/test/browser/useId.test.js | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index 3310b13c93..1a2f521b91 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -316,7 +316,7 @@ export function useId() { const state = getHookState(currentIndex++, 11); if (!state._id) { currentIdCounter++; - state._id = '' + currentInternal._rootId + currentInternal._domDepth + currentIdCounter + state._id = (currentInternal._rootId + currentInternal._domDepth + currentIdCounter).toString(32) } return state._id; } diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index 355f69dad5..76e6560bc6 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -52,4 +52,29 @@ describe('useId', () => { render(, scratch); expect(scratch.innerHTML).to.equal('
h
') }); + + // TODO: ensure uniqueness across siblings (could do so by checking the index of the internal within the parentChildren) + it.skip('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('') + + render(, scratch); + expect(scratch.innerHTML).to.equal('') + }); }); From ecfa60b73372d044035ca7a56ce234b6c0ef0255 Mon Sep 17 00:00:00 2001 From: JoviDeCroock Date: Mon, 10 Jan 2022 16:36:57 +0100 Subject: [PATCH 08/16] use a global counter instead --- hooks/src/index.js | 1 - hooks/test/browser/useId.test.js | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index 1a2f521b91..1fae8ca8fb 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -35,7 +35,6 @@ options._render = internal => { if (oldBeforeRender) oldBeforeRender(internal); currentInternal = internal; - currentIdCounter = 0; currentIndex = 0; if (currentInternal.data && currentInternal.data.__hooks) { diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index 76e6560bc6..b5c1a087ba 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -53,8 +53,7 @@ describe('useId', () => { expect(scratch.innerHTML).to.equal('
h
') }); - // TODO: ensure uniqueness across siblings (could do so by checking the index of the internal within the parentChildren) - it.skip('ids are unique across siblings', () => { + it('ids are unique across siblings', () => { function Child() { const id = useId() return h From 832c49737c4e7f177f7184adaddeabeba4c14602 Mon Sep 17 00:00:00 2001 From: JoviDeCroock Date: Mon, 10 Jan 2022 16:39:29 +0100 Subject: [PATCH 09/16] start on 0 --- 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 1fae8ca8fb..317aa84705 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -12,7 +12,7 @@ let currentInternal; let currentHook = 0; /** @type {number} */ -let currentIdCounter; +let currentIdCounter = 0; /** @type {Array} */ let afterPaintEffects = []; From 564489580e73adb74c75da25cba65e3ef75dc14a Mon Sep 17 00:00:00 2001 From: JoviDeCroock Date: Mon, 10 Jan 2022 16:42:18 +0100 Subject: [PATCH 10/16] add test assertions --- hooks/test/browser/useId.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index b5c1a087ba..d1ac12d41c 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -47,10 +47,10 @@ describe('useId', () => { } render(, scratch); - expect(scratch.innerHTML).to.equal('
h
') + expect(scratch.innerHTML).to.equal('
h
') render(, scratch); - expect(scratch.innerHTML).to.equal('
h
') + expect(scratch.innerHTML).to.equal('
h
') }); it('ids are unique across siblings', () => { @@ -71,9 +71,9 @@ describe('useId', () => { } render(, scratch); - expect(scratch.innerHTML).to.equal('') + expect(scratch.innerHTML).to.equal('
hhh
') render(, scratch); - expect(scratch.innerHTML).to.equal('') + expect(scratch.innerHTML).to.equal('
hhh
') }); }); From d015cbbdf59db4214b3dd039f60ac6fecbef705e Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 20 Mar 2022 11:27:53 +0100 Subject: [PATCH 11/16] make useId more consistent --- hooks/src/index.js | 10 +++++-- hooks/test/browser/useId.test.js | 46 ++++++++++++++++++++------------ mangle.json | 1 - src/internal.d.ts | 2 -- src/tree.js | 4 +-- 5 files changed, 38 insertions(+), 25 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index c6f9c6d3b1..6c0c0ea8bb 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -351,12 +351,18 @@ export function useErrorBoundary(cb) { ]; } - export function useId() { const state = getHookState(currentIndex++, 11); if (!state._id) { currentIdCounter++; - state._id = (currentInternal._rootId + currentInternal._domDepth + currentIdCounter).toString(32) + + state._id = + '_P' + + ( + currentInternal._rootId + + currentInternal._parent._children.indexOf(currentInternal) + + currentIdCounter + ).toString(32); } return state._id; } diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index d1ac12d41c..197eab88cb 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -4,7 +4,7 @@ import { useId } from 'preact/hooks'; /** @jsx createElement */ -describe('useId', () => { +describe.only('useId', () => { /** @type {HTMLDivElement} */ let scratch; @@ -18,49 +18,57 @@ describe('useId', () => { it('keeps the id consistent after an update', () => { function Comp() { - const id = useId() + const id = useId(); return
; } render(, scratch); - const id = scratch.firstChild.getAttribute('id') - expect(scratch.firstChild.getAttribute('id')).to.equal(id) + const id = scratch.firstChild.getAttribute('id'); + expect(scratch.firstChild.getAttribute('id')).to.equal(id); render(, scratch); - expect(scratch.firstChild.getAttribute('id')).to.equal(id) + expect(scratch.firstChild.getAttribute('id')).to.equal(id); }); it('ids are unique according to dom-depth', () => { function Child() { - const id = useId() - const spanId = useId() + const id = useId(); + const spanId = useId(); return (
h
- ) + ); } function Comp() { - const id = useId() - return
; + const id = useId(); + return ( +
+ +
+ ); } render(, scratch); - expect(scratch.innerHTML).to.equal('
h
') + expect(scratch.innerHTML).to.equal( + '
h
' + ); render(, scratch); - expect(scratch.innerHTML).to.equal('
h
') + expect(scratch.innerHTML).to.equal( + '
h
' + ); }); it('ids are unique across siblings', () => { function Child() { - const id = useId() - return h + const id = useId(); + return h; } function Comp() { - const id = useId() + const id = useId(); return (
@@ -71,9 +79,13 @@ describe('useId', () => { } render(, scratch); - expect(scratch.innerHTML).to.equal('
hhh
') + expect(scratch.innerHTML).to.equal( + '
hhh
' + ); render(, scratch); - expect(scratch.innerHTML).to.equal('
hhh
') + expect(scratch.innerHTML).to.equal( + '
hhh
' + ); }); }); diff --git a/mangle.json b/mangle.json index b223bfb1a0..109cfe2578 100644 --- a/mangle.json +++ b/mangle.json @@ -44,7 +44,6 @@ "$_args": "__H", "$_factory": "__h", "$_depth": "__b", - "$_domDepth": "__dd", "$_detachOnNextRender": "__b", "$_nextState": "__s", "$_commitCallbacks": "__h", diff --git a/src/internal.d.ts b/src/internal.d.ts index ad87770106..a283d3f49b 100644 --- a/src/internal.d.ts +++ b/src/internal.d.ts @@ -168,8 +168,6 @@ export interface Internal

{ _context: any; /** This Internal's distance from the tree root counted in vdom-nodes */ _depth: number | null; - /** This Internal's distance from the tree root counted in dom-nodes */ - _domDepth: number | null; /** The id of the root */ _rootId: number | null; /** Callbacks to invoke when this internal commits */ diff --git a/src/tree.js b/src/tree.js index 2645a9353b..077c197e16 100644 --- a/src/tree.js +++ b/src/tree.js @@ -81,7 +81,6 @@ export function createInternal(vnode, parentInternal, rootId) { } } - const parentDomDepth = parentInternal ? parentInternal._domDepth : 0; /** @type {import('./internal').Internal} */ const internal = { type, @@ -100,8 +99,7 @@ export function createInternal(vnode, parentInternal, rootId) { _component: null, _context: null, _depth: parentInternal ? parentInternal._depth + 1 : 0, - _rootId: rootId || (parentInternal && parentInternal._rootId || 0), - _domDepth: typeof type == 'function' ? parentDomDepth : parentDomDepth + 1 + _rootId: rootId || ((parentInternal && parentInternal._rootId) || 0) }; if (options._internal) options._internal(internal, vnode); From 7cfe8841b9a3593dea24bfdd68ba1c7b1b053ebe Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Sun, 20 Mar 2022 11:29:17 +0100 Subject: [PATCH 12/16] Update useId.test.js --- hooks/test/browser/useId.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index 197eab88cb..a5f6eadd86 100644 --- a/hooks/test/browser/useId.test.js +++ b/hooks/test/browser/useId.test.js @@ -4,7 +4,7 @@ import { useId } from 'preact/hooks'; /** @jsx createElement */ -describe.only('useId', () => { +describe('useId', () => { /** @type {HTMLDivElement} */ let scratch; From ab2ed5da84765a4c2e334d67134b59cd51495a90 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 20 Mar 2022 11:33:09 +0100 Subject: [PATCH 13/16] start rootId at 1 --- hooks/test/browser/useId.test.js | 8 ++++---- src/create-root.js | 2 +- src/tree.js | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index a5f6eadd86..c2180b7686 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
' ); }); }); diff --git a/src/create-root.js b/src/create-root.js index 8d45926a66..d2656eadd5 100644 --- a/src/create-root.js +++ b/src/create-root.js @@ -12,7 +12,7 @@ import { patch } from './diff/patch'; import { createInternal } from './tree'; import { rendererState } from './component'; -let rootId = 0; +let rootId = 1; /** * diff --git a/src/tree.js b/src/tree.js index 077c197e16..3d1ed3f77c 100644 --- a/src/tree.js +++ b/src/tree.js @@ -99,7 +99,7 @@ export function createInternal(vnode, parentInternal, rootId) { _component: null, _context: null, _depth: parentInternal ? parentInternal._depth + 1 : 0, - _rootId: rootId || ((parentInternal && parentInternal._rootId) || 0) + _rootId: rootId || (parentInternal && parentInternal._rootId) }; if (options._internal) options._internal(internal, vnode); From e9c1876870df9eede0fb48b0a394e0519fbc5957 Mon Sep 17 00:00:00 2001 From: jdecroock Date: Sun, 20 Mar 2022 11:50:05 +0100 Subject: [PATCH 14/16] namespace by rootId and add depth --- hooks/src/index.js | 9 ++++----- hooks/test/browser/useId.test.js | 8 ++++---- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index 6c0c0ea8bb..b9e5c8b002 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -358,11 +358,10 @@ export function useId() { state._id = '_P' + - ( - currentInternal._rootId + - currentInternal._parent._children.indexOf(currentInternal) + - currentIdCounter - ).toString(32); + currentInternal._rootId + + (currentInternal._parent._children.indexOf(currentInternal) + + currentInternal._depth + + currentIdCounter); } return state._id; } diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index c2180b7686..c85ff85020 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 0db21609c5a7228a8b89ef94e3f550d5a257739b Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Fri, 29 Apr 2022 21:53:16 +0100 Subject: [PATCH 15/16] remove rootId --- hooks/src/index.js | 1 - hooks/test/browser/useId.test.js | 8 ++++---- src/create-root.js | 4 +--- src/internal.d.ts | 2 -- src/tree.js | 6 ++---- 5 files changed, 7 insertions(+), 14 deletions(-) diff --git a/hooks/src/index.js b/hooks/src/index.js index d82cf70757..4c96a02fb9 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -363,7 +363,6 @@ export function useId() { state._id = '_P' + - currentInternal._rootId + (currentInternal._parent._children.indexOf(currentInternal) + currentInternal._depth + currentIdCounter); diff --git a/hooks/test/browser/useId.test.js b/hooks/test/browser/useId.test.js index c85ff85020..2f86a31b12 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
' ); }); }); diff --git a/src/create-root.js b/src/create-root.js index d2656eadd5..8798830b9e 100644 --- a/src/create-root.js +++ b/src/create-root.js @@ -12,8 +12,6 @@ import { patch } from './diff/patch'; import { createInternal } from './tree'; import { rendererState } from './component'; -let rootId = 1; - /** * * @param {import('./internal').PreactElement} parentDom The DOM element to @@ -40,7 +38,7 @@ export function createRoot(parentDom) { if (rootInternal) { patch(rootInternal, vnode); } else { - rootInternal = createInternal(vnode, undefined, rootId); + rootInternal = createInternal(vnode, undefined); // Store the VDOM tree root on the DOM element in a (minified) property: parentDom._children = rootInternal; diff --git a/src/internal.d.ts b/src/internal.d.ts index a283d3f49b..1de1aa8137 100644 --- a/src/internal.d.ts +++ b/src/internal.d.ts @@ -168,8 +168,6 @@ export interface Internal

{ _context: any; /** This Internal's distance from the tree root counted in vdom-nodes */ _depth: number | null; - /** The id of the root */ - _rootId: number | null; /** Callbacks to invoke when this internal commits */ _commitCallbacks: Array<() => void>; } diff --git a/src/tree.js b/src/tree.js index 3d1ed3f77c..82a3103e10 100644 --- a/src/tree.js +++ b/src/tree.js @@ -17,10 +17,9 @@ import { enqueueRender } from './component'; * Create an internal tree node * @param {import('./internal').VNode | string} vnode * @param {import('./internal').Internal} [parentInternal] - * @param {number} [rootId] * @returns {import('./internal').Internal} */ -export function createInternal(vnode, parentInternal, rootId) { +export function createInternal(vnode, parentInternal) { let type = null, props, key, @@ -98,8 +97,7 @@ export function createInternal(vnode, parentInternal, rootId) { _dom: null, _component: null, _context: null, - _depth: parentInternal ? parentInternal._depth + 1 : 0, - _rootId: rootId || (parentInternal && parentInternal._rootId) + _depth: parentInternal ? parentInternal._depth + 1 : 0 }; if (options._internal) options._internal(internal, vnode); From d1c4bf10cce1ff4d8c2b0eea476dd680f46353cd Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Sun, 1 May 2022 10:33:59 +0100 Subject: [PATCH 16/16] try server impl --- hooks/src/index.js | 4 + package-lock.json | 1 + server/jsx.js | 2 +- server/src/index.d.ts | 22 ++---- server/src/index.js | 141 +++++++++++++++++++++++------------- server/src/jsx.d.ts | 13 ++++ server/src/jsx.js | 7 +- server/src/util.js | 12 +++ server/test/context.test.js | 2 +- server/test/jsx.test.js | 2 +- server/test/pretty.test.js | 14 ++-- server/test/render.test.js | 64 +++++++++++++++- 12 files changed, 203 insertions(+), 81 deletions(-) create mode 100644 server/src/jsx.d.ts diff --git a/hooks/src/index.js b/hooks/src/index.js index 4c96a02fb9..5c2de2d610 100644 --- a/hooks/src/index.js +++ b/hooks/src/index.js @@ -361,6 +361,10 @@ export function useId() { if (!state._id) { currentIdCounter++; + console.log( + currentInternal, + currentInternal._parent._children.indexOf(currentInternal) + ); state._id = '_P' + (currentInternal._parent._children.indexOf(currentInternal) + diff --git a/package-lock.json b/package-lock.json index 098b777883..93058167f1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5,6 +5,7 @@ "requires": true, "packages": { "": { + "name": "preact", "version": "10.5.13", "license": "MIT", "dependencies": { diff --git a/server/jsx.js b/server/jsx.js index 1ef4f671bb..e1e483a795 100644 --- a/server/jsx.js +++ b/server/jsx.js @@ -1 +1 @@ -module.exports = require('./dist/jsx'); // eslint-disable-line +module.exports = require('./dist/server'); // eslint-disable-line diff --git a/server/src/index.d.ts b/server/src/index.d.ts index b242fcca6d..221d349a1b 100644 --- a/server/src/index.d.ts +++ b/server/src/index.d.ts @@ -1,26 +1,16 @@ import { VNode } from 'preact'; -export interface Options { +interface Options { shallow?: boolean; xml?: boolean; pretty?: boolean | string; } -type RenderFn = (vnode: VNode, context?: any, options?: Options) => string; -export const render: RenderFn; -export const renderToString: RenderFn; -export const renderToStaticMarkup: RenderFn; - -export function shallowRender(vnode: VNode, context?: any): string; - -export interface JsxOptions extends Options { - functions?: boolean; - functionNames?: boolean; - skipFalseAttributes?: boolean; -} - -export function renderToJsxString( +export function render(vnode: VNode, context?: any, options?: Options): string; +export function renderToString( vnode: VNode, context?: any, - options: JsxOptions + options?: Options ): string; +export function shallowRender(vnode: VNode, context?: any): string; +export default render; diff --git a/server/src/index.js b/server/src/index.js index ee49b50c34..26eec9ba1d 100644 --- a/server/src/index.js +++ b/server/src/index.js @@ -4,9 +4,10 @@ import { isLargeString, styleObjToCss, assign, - getChildren + getChildren, + createInternalFromVnode } from './util'; -import { renderToJsxString } from './jsx'; +import renderToJsxString from './jsx'; import { options, Fragment } from 'preact'; /** @typedef {import('preact').VNode} VNode */ @@ -22,30 +23,29 @@ const UNSAFE_NAME = /[\s\n\\/='"\0<>]/; const noop = () => {}; -/** - * Only render elements, leaving Components inline as ``. +/** Render Preact JSX + Components to an HTML string. + * @name render + * @function + * @param {VNode} vnode JSX VNode to render. + * @param {Object} [context={}] Optionally pass an initial context object through the render path. + * @param {Object} [options={}] Rendering options + * @param {Boolean} [options.shallow=false] If `true`, renders nested Components as HTML elements (``). + * @param {Boolean} [options.xml=false] If `true`, uses self-closing tags for elements without children. + * @param {Boolean} [options.pretty=false] If `true`, adds whitespace for readability + * @param {RegExp|undefined} [options.voidElements] RegeEx that matches elements that are considered void (self-closing) + */ +renderToString.render = renderToString; + +/** Only render elements, leaving Components inline as ``. * This method is just a convenience alias for `render(vnode, context, { shallow:true })` - * @name shallow - * @function - * @param {VNode} vnode JSX VNode to render. - * @param {Object} [context={}] Optionally pass an initial context object through the render path. + * @name shallow + * @function + * @param {VNode} vnode JSX VNode to render. + * @param {Object} [context={}] Optionally pass an initial context object through the render path. */ let shallowRender = (vnode, context) => renderToString(vnode, context, SHALLOW); const EMPTY_ARR = []; - -/** - * Render Preact JSX + Components to an HTML string. - * @name render - * @function - * @param {VNode} vnode JSX VNode to render. - * @param {Object} [context={}] Optionally pass an initial context object through the render path. - * @param {Object} [opts={}] Rendering options - * @param {Boolean} [opts.shallow=false] If `true`, renders nested Components as HTML elements (``). - * @param {Boolean} [opts.xml=false] If `true`, uses self-closing tags for elements without children. - * @param {Boolean} [opts.pretty=false] If `true`, adds whitespace for readability - * @param {RegExp|undefined} [opts.voidElements] RegeEx that matches elements that are considered void (self-closing) - */ function renderToString(vnode, context, opts) { context = context || {}; opts = opts || {}; @@ -55,21 +55,38 @@ function renderToString(vnode, context, opts) { // array to `options._commit` (`__c`). But we can go one step further // and avoid a lot of dirty checks and allocations by setting // `options._skipEffects` (`__s`) too. - const previousSkipEffects = options._skipEffects; - options._skipEffects = true; - - const res = _renderToString(vnode, context, opts); + const previousSkipEffects = options.__s; + options.__s = true; + + const internal = { _children: [], _depth: 1 }; + const res = _renderToString( + vnode, + context, + opts, + undefined, + undefined, + undefined, + internal + ); // options._commit, we don't schedule any effects in this library right now, // so we can pass an empty queue to this hook. - if (options._commit) options._commit(vnode, EMPTY_ARR); + if (options.__c) options.__c(vnode, EMPTY_ARR); EMPTY_ARR.length = 0; - options._skipEffects = previousSkipEffects; + options.__s = previousSkipEffects; return res; } /** The default export is an alias of `render()`. */ -function _renderToString(vnode, context, opts, inner, isSvgMode, selectValue) { +function _renderToString( + vnode, + context, + opts, + inner, + isSvgMode, + selectValue, + parent +) { if (vnode == null || typeof vnode === 'boolean') { return ''; } @@ -86,13 +103,16 @@ function _renderToString(vnode, context, opts, inner, isSvgMode, selectValue) { let rendered = ''; for (let i = 0; i < vnode.length; i++) { if (pretty && i > 0) rendered += '\n'; + const internal = createInternalFromVnode(vnode, context, parent); + parent._children.push(internal); rendered += _renderToString( vnode[i], context, opts, inner, isSvgMode, - selectValue + selectValue, + internal ); } return rendered; @@ -110,35 +130,39 @@ function _renderToString(vnode, context, opts, inner, isSvgMode, selectValue) { } else if (nodeName === Fragment) { const children = []; getChildren(children, vnode.props.children); + const internal = createInternalFromVnode(vnode, context, parent); + parent._children.push(internal); return _renderToString( children, context, opts, opts.shallowHighOrder !== false, isSvgMode, - selectValue + selectValue, + internal ); } else { let rendered; - vnode._context = context; - vnode.data = { - _hooks: [] - }; - - let c = (vnode._component = { + let c = (vnode.__c = { + __v: vnode, context, props: vnode.props, // silently drop state updates setState: noop, - forceUpdate: noop + forceUpdate: noop, + // hooks + data: {} }); // options._diff - if (options._diff) options._diff(vnode); + if (options.__b) options.__b(vnode); + + const internal = createInternalFromVnode(vnode, context, parent); + parent._children.push(internal); // options._render - if (options._render) options._render(vnode); + if (options.__r) options.__r(internal); if ( !nodeName.prototype || @@ -147,36 +171,37 @@ function _renderToString(vnode, context, opts, inner, isSvgMode, selectValue) { // Necessary for createContext api. Setting this property will pass // the context value as `this.context` just for this component. let cxType = nodeName.contextType; - let provider = cxType && context[cxType._id]; + let provider = cxType && context[cxType.__c]; let cctx = cxType != null ? provider ? provider.props.value - : cxType._defaultValue + : cxType.__ : context; // stateless functional components - rendered = nodeName.call(vnode._component, props, cctx); + rendered = nodeName.call(vnode.__c, props, cctx); } else { // class-based components let cxType = nodeName.contextType; - let provider = cxType && context[cxType._id]; + let provider = cxType && context[cxType.__c]; let cctx = cxType != null ? provider ? provider.props.value - : cxType._defaultValue + : cxType.__ : context; // c = new nodeName(props, context); - c = vnode._component = new nodeName(props, cctx); + c = vnode.__c = new nodeName(props, cctx); + c.__v = vnode; // turn off stateful re-rendering: - c._dirty = true; + c._dirty = c.__d = true; c.props = props; if (c.state == null) c.state = {}; - if (c._nextState == null) { - c._nextState = c.state; + if (c._nextState == null && c.__s == null) { + c._nextState = c.__s = c.state; } c.context = cctx; @@ -190,7 +215,12 @@ function _renderToString(vnode, context, opts, inner, isSvgMode, selectValue) { // If the user called setState in cWM we need to flush pending, // state updates. This is the same behaviour in React. - c.state = c._nextState !== c.state ? c._nextState : c.state; + c.state = + c._nextState !== c.state + ? c._nextState + : c.__s !== c.state + ? c.__s + : c.state; } rendered = c.render(c.props, c.state, c.context); @@ -207,7 +237,8 @@ function _renderToString(vnode, context, opts, inner, isSvgMode, selectValue) { opts, opts.shallowHighOrder !== false, isSvgMode, - selectValue + selectValue, + internal ); } } @@ -337,6 +368,8 @@ function _renderToString(vnode, context, opts, inner, isSvgMode, selectValue) { let child = children[i]; if (child != null && child !== false) { + const internal = createInternalFromVnode(vnode, context, parent); + parent._children.push(internal); let childSvgMode = nodeName === 'svg' ? true @@ -349,7 +382,8 @@ function _renderToString(vnode, context, opts, inner, isSvgMode, selectValue) { opts, true, childSvgMode, - selectValue + selectValue, + internal ); if (pretty && !hasLarge && isLargeString(ret)) hasLarge = true; @@ -425,11 +459,14 @@ function getFallbackComponentName(component) { } return name; } +renderToString.shallowRender = shallowRender; + +export default renderToString; export { renderToString as render, renderToString as renderToStaticMarkup, - renderToString, renderToJsxString, + renderToString, shallowRender }; diff --git a/server/src/jsx.d.ts b/server/src/jsx.d.ts new file mode 100644 index 0000000000..98fad5553e --- /dev/null +++ b/server/src/jsx.d.ts @@ -0,0 +1,13 @@ +import { VNode } from 'preact'; + +interface Options { + jsx?: boolean; + xml?: boolean; + functions?: boolean; + functionNames?: boolean; + skipFalseAttributes?: boolean; + pretty?: boolean | string; +} + +export function render(vnode: VNode, context?: any, options?: Options): string; +export default render; diff --git a/server/src/jsx.js b/server/src/jsx.js index c37bc5d1be..f2ac1c7853 100644 --- a/server/src/jsx.js +++ b/server/src/jsx.js @@ -1,5 +1,5 @@ import './polyfills'; -import { renderToString } from './index'; +import renderToString from './index'; import { indent, encodeEntities, assign } from './util'; import prettyFormat from 'pretty-format'; @@ -67,7 +67,10 @@ let defaultOpts = { pretty: ' ' }; -export function renderToJsxString(vnode, context, opts, inner) { +function renderToJsxString(vnode, context, opts, inner) { opts = assign(assign({}, defaultOpts), opts || {}); return renderToString(vnode, context, opts, inner); } + +export default renderToJsxString; +export { renderToJsxString as render }; diff --git a/server/src/util.js b/server/src/util.js index 5ade429e3c..40838a2f53 100644 --- a/server/src/util.js +++ b/server/src/util.js @@ -76,3 +76,15 @@ export function getChildren(accumulator, children) { } return accumulator; } + +export function createInternalFromVnode(vnode, context, parent, depth) { + return { + type: vnode.type, + props: vnode.props, + data: {}, + c: context, + _children: [], + _parent: parent, + _depth: parent._depth + 1 + }; +} diff --git a/server/test/context.test.js b/server/test/context.test.js index 7e475074d7..cbdddb5174 100644 --- a/server/test/context.test.js +++ b/server/test/context.test.js @@ -1,4 +1,4 @@ -import { renderToJsxString as render } from '../src/jsx'; +import render from '../src/jsx'; import { createElement, createContext, Component } from 'preact'; import { expect } from 'chai'; import { dedent } from './utils'; diff --git a/server/test/jsx.test.js b/server/test/jsx.test.js index aa47b5a4ca..40dfb59bff 100644 --- a/server/test/jsx.test.js +++ b/server/test/jsx.test.js @@ -1,4 +1,4 @@ -import { renderToJsxString as render } from '../src/jsx'; +import render from '../src/jsx'; import { createElement } from 'preact'; import { expect } from 'chai'; import { dedent } from './utils'; diff --git a/server/test/pretty.test.js b/server/test/pretty.test.js index 2694d41079..1b054a39b7 100644 --- a/server/test/pretty.test.js +++ b/server/test/pretty.test.js @@ -1,5 +1,5 @@ import { render as basicRender } from '../src'; -import { renderToJsxString as render } from '../src/jsx'; +import render from '../src/jsx'; import { createElement, Fragment } from 'preact'; import { expect } from 'chai'; import { dedent } from './utils'; @@ -123,7 +123,7 @@ describe('pretty', () => {

hello{' '}
)).to.equal(dedent`
- hello + hello
`); @@ -133,7 +133,7 @@ describe('pretty', () => {
hello{' '} {'a'}{'b'}
)).to.equal(dedent`
- hello + hello ab
@@ -146,7 +146,7 @@ describe('pretty', () => {
foobar{' '}
)).to.equal(dedent`
- foobar + foobar
`); @@ -174,7 +174,7 @@ describe('pretty', () => { ) ).to.equal(dedent`

- a + a b

`); @@ -201,7 +201,7 @@ describe('pretty', () => { ) ).to.equal(dedent`

- a\ + a\ b

`); @@ -219,7 +219,7 @@ describe('pretty', () => { ).to.equal(dedent`

- \ a\ + \ a\

`); }); diff --git a/server/test/render.test.js b/server/test/render.test.js index 125c0948fe..7dfa7edead 100644 --- a/server/test/render.test.js +++ b/server/test/render.test.js @@ -7,7 +7,13 @@ import { Fragment, options } from 'preact'; -import { useState, useContext, useEffect, useLayoutEffect } from 'preact/hooks'; +import { + useState, + useContext, + useEffect, + useLayoutEffect, + useId +} from 'preact/hooks'; import { expect } from 'chai'; import { spy, stub, match } from 'sinon'; import './setup'; @@ -1189,4 +1195,60 @@ describe('render', () => { '' ); }); + + describe('useId', () => { + it.only('ids are unique according to dom-depth', () => { + function Child() { + const id = useId(); + const spanId = useId(); + return ( +
+ h +
+ ); + } + + function Comp() { + const id = useId(); + return ( +
+ +
+ ); + } + + let res = render(); + expect(res).to.equal( + '
h
' + ); + + res = render(); + expect(res).to.equal( + '
h
' + ); + }); + + it('ids are unique across siblings', () => { + function Child() { + const id = useId(); + return h; + } + + function Comp() { + const id = useId(); + return ( +
+ + + +
+ ); + } + + const res = render(); + expect(res).to.equal( + '
hhh
' + ); + }); + }); });