From aa12b3c61528813c7a3978410d1d551afbdb08ba Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Thu, 6 Oct 2022 20:48:43 +0200 Subject: [PATCH 1/2] Fix useId mismatch due to top level Fragments --- .changeset/few-elephants-occur.md | 5 +++ package-lock.json | 38 ++++++++------------- package.json | 2 +- src/index.js | 6 ++++ test/render.test.js | 55 ++++++++++++++++++++++++++++++- 5 files changed, 80 insertions(+), 26 deletions(-) create mode 100644 .changeset/few-elephants-occur.md diff --git a/.changeset/few-elephants-occur.md b/.changeset/few-elephants-occur.md new file mode 100644 index 00000000..5156e3e6 --- /dev/null +++ b/.changeset/few-elephants-occur.md @@ -0,0 +1,5 @@ +--- +'preact-render-to-string': patch +--- + +Fix vnode masks not matching with core due to top level component Fragments diff --git a/package-lock.json b/package-lock.json index 079e9c20..72750830 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,11 @@ { "name": "preact-render-to-string", - "version": "5.2.1", + "version": "5.2.4", "lockfileVersion": 2, "requires": true, "packages": { "": { - "name": "preact-render-to-string", - "version": "5.2.1", + "version": "5.2.4", "license": "MIT", "dependencies": { "pretty-format": "^3.8.0" @@ -17,7 +16,6 @@ "@babel/register": "^7.12.10", "@changesets/changelog-github": "^0.4.1", "@changesets/cli": "^2.18.0", - "benchmarkjs": "^0.1.8", "benchmarkjs-pretty": "^2.0.1", "chai": "^4.2.0", "copyfiles": "^2.4.1", @@ -27,7 +25,7 @@ "lint-staged": "^10.5.3", "microbundle": "^0.13.0", "mocha": "^8.2.1", - "preact": "^10.5.7", + "preact": "^10.11.1", "prettier": "^2.2.1", "sinon": "^9.2.2", "sinon-chai": "^3.5.0", @@ -2629,12 +2627,6 @@ "platform": "^1.3.3" } }, - "node_modules/benchmarkjs": { - "version": "0.1.8", - "resolved": "https://registry.npmjs.org/benchmarkjs/-/benchmarkjs-0.1.8.tgz", - "integrity": "sha512-0NBdYmBvCe5EdKRYiRk0op3MzbnEcLz/3kjKl1BD5b8q0HmzeHOs10lwZXJoC95sfgQOnxBTBNbbe1t56yvYpg==", - "dev": true - }, "node_modules/benchmarkjs-pretty": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/benchmarkjs-pretty/-/benchmarkjs-pretty-2.0.1.tgz", @@ -10671,10 +10663,14 @@ } }, "node_modules/preact": { - "version": "10.5.7", - "resolved": "https://registry.npmjs.org/preact/-/preact-10.5.7.tgz", - "integrity": "sha512-4oEpz75t/0UNcwmcsjk+BIcDdk68oao+7kxcpc1hQPNs2Oo3ZL9xFz8UBf350mxk/VEdD41L5b4l2dE3Ug3RYg==", - "dev": true + "version": "10.11.1", + "resolved": "https://registry.npmjs.org/preact/-/preact-10.11.1.tgz", + "integrity": "sha512-1Wz5PCRm6Fg+6BTXWJHhX4wRK9MZbZBHuwBqfZlOdVm2NqPe8/rjYpufvYCwJSGb9layyzB2jTTXfpCTynLqFQ==", + "dev": true, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/preact" + } }, "node_modules/preferred-pm": { "version": "3.0.3", @@ -15704,12 +15700,6 @@ "platform": "^1.3.3" } }, - "benchmarkjs": { - "version": "0.1.8", - "resolved": "https://registry.npmjs.org/benchmarkjs/-/benchmarkjs-0.1.8.tgz", - "integrity": "sha512-0NBdYmBvCe5EdKRYiRk0op3MzbnEcLz/3kjKl1BD5b8q0HmzeHOs10lwZXJoC95sfgQOnxBTBNbbe1t56yvYpg==", - "dev": true - }, "benchmarkjs-pretty": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/benchmarkjs-pretty/-/benchmarkjs-pretty-2.0.1.tgz", @@ -22222,9 +22212,9 @@ "dev": true }, "preact": { - "version": "10.5.7", - "resolved": "https://registry.npmjs.org/preact/-/preact-10.5.7.tgz", - "integrity": "sha512-4oEpz75t/0UNcwmcsjk+BIcDdk68oao+7kxcpc1hQPNs2Oo3ZL9xFz8UBf350mxk/VEdD41L5b4l2dE3Ug3RYg==", + "version": "10.11.1", + "resolved": "https://registry.npmjs.org/preact/-/preact-10.11.1.tgz", + "integrity": "sha512-1Wz5PCRm6Fg+6BTXWJHhX4wRK9MZbZBHuwBqfZlOdVm2NqPe8/rjYpufvYCwJSGb9layyzB2jTTXfpCTynLqFQ==", "dev": true }, "preferred-pm": { diff --git a/package.json b/package.json index f6296ac6..3d75aa0b 100644 --- a/package.json +++ b/package.json @@ -115,7 +115,7 @@ "lint-staged": "^10.5.3", "microbundle": "^0.13.0", "mocha": "^8.2.1", - "preact": "^10.5.7", + "preact": "^10.11.1", "prettier": "^2.2.1", "sinon": "^9.2.2", "sinon-chai": "^3.5.0", diff --git a/src/index.js b/src/index.js index 4f5e2831..946610ff 100644 --- a/src/index.js +++ b/src/index.js @@ -239,6 +239,12 @@ function _renderToString(vnode, context, isSvgMode, selectValue, parent) { } } + // When a component returns a Fragment node we flatten it in core, so we + // need to mirror that logic here too + let isTopLevelFragment = + rendered != null && rendered.type === Fragment && rendered.key == null; + rendered = isTopLevelFragment ? rendered.props.children : rendered; + // Recurse into children before invoking the after-diff hook const str = _renderToString( rendered, diff --git a/test/render.test.js b/test/render.test.js index 8f234821..8e90a910 100644 --- a/test/render.test.js +++ b/test/render.test.js @@ -5,7 +5,8 @@ import { useContext, useEffect, useLayoutEffect, - useMemo + useMemo, + useId } from 'preact/hooks'; import { expect } from 'chai'; import { spy, stub, match } from 'sinon'; @@ -1268,4 +1269,56 @@ describe('render', () => { it('should not render function children', () => { expect(render(
{() => {}}
)).to.equal('
'); }); + + describe('vnode masks (useId)', () => { + it('should work with Fragments', () => { + const ids = []; + function Foo() { + const id = useId(); + ids.push(id); + return

{id}

; + } + + function Bar(props) { + return props.children; + } + + function App() { + return ( + + + + + + + ); + } + + render(); + expect(ids[0]).not.to.equal(ids[1]); + }); + + it('should skip component top level Fragment child', () => { + const Wrapper = ({ children }) => {children}; + + function Foo() { + const id = useId(); + return

{id}

; + } + + function App() { + const id = useId(); + return ( +
+

{id}

+ + + +
+ ); + } + + expect(render()).to.equal('

P481

P476951

'); + }); + }); }); From 9ec2dc97b7dc3eaaf277929b3a813315880a65ef Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Thu, 6 Oct 2022 21:04:33 +0200 Subject: [PATCH 2/2] Remove copied test from core --- test/render.test.js | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/test/render.test.js b/test/render.test.js index 8e90a910..7e3dc78d 100644 --- a/test/render.test.js +++ b/test/render.test.js @@ -1271,33 +1271,6 @@ describe('render', () => { }); describe('vnode masks (useId)', () => { - it('should work with Fragments', () => { - const ids = []; - function Foo() { - const id = useId(); - ids.push(id); - return

{id}

; - } - - function Bar(props) { - return props.children; - } - - function App() { - return ( - - - - - - - ); - } - - render(); - expect(ids[0]).not.to.equal(ids[1]); - }); - it('should skip component top level Fragment child', () => { const Wrapper = ({ children }) => {children};