[labs/nextjs, labs/ssr-react, lit/react] Add support for Next.js v14 and App Router #4575

augustjk · 2024-03-09T01:28:06Z

Apologies for the big PR. It'll be easier to review commit by commit.

Explanation of changes:

re: f10d7ee

Next.js v14 had some changes to how it bundles its copy of React so how we were hijacking imports of react/jsx-runtime and react/jsx-dev-runtime with the Webpack module replacement were not working described here vercel/next.js#46470 (comment)

Since React is still all CommonJS, I've resorted to monkey-patching everything including the runtime JSX functions. Also, a workaround in how we access the module to patch was added to handle seemingly inconsistent es module interop behavior in webpack, e.g. (React.default || React).createElement = .... This addresses errors like seen here #3657 (comment)

This has a nice benefit that all the hijacking/patching is possible with just a single import of @lit-labs/ssr-react/enable-lit-ssr.js which allows the @lit-labs/nextjs plugin to be a lot simpler (see next commit). Users can also choose not to use the plugin and just add this one line import themselves. It should also make integration with other React meta-framework, namely Remix, easier (need to test this).

I also added a small check to make sure we don't wrap createElement and the jsx functions if they're already wrapped. This is mainly useful for when both @lit-labs/ssr-react/enable-lit-ssr.js is installed, and the "jsxImportSource": "@lit-labs/ssr-react" is set, as it happens in our integration test suite. This doesn't seem to actually be a problem.

These changes only affect @lit-labs/ssr-react package.

re: 9702f91

With the changes above, the plugin can become a lot simpler. I've removed all the hacky webpack externals modification to replace react/runtime-jsx at the module level since we're just monkey-patching it globally now.

I've updated the peer dependencies to now support next@13 and next@14, and dropped next@12. I debated whether we should keep v12 support, but the changes here mainly apply to next v13 and v14 for the app router support and I haven't tested these changes in v12 (and we likely won't be going forward, see commit about adding example projects below). Users of v12 can stick with the current published version.

re: d5dcabb

While manually testing the Next v14 example, I noticed server rendering of @lit/react wrapped components had incorrect initial props for production builds. We had coordination between @lit/react and @lit-labs/ssr-react based on the patched createElement function name, but that name check was failing perhaps as it got lost during production build minification. (I noticed this a while back when I was testing out @lit-labs/ssr-react in Remix too.) @lit-labs/ssr-react will now add a global flag which @lit/react node build will check for.

re: 02a0c05

I renamed the existing examples/nextjs package to nextjs-v13 and added nextjs-v14 and nextjs-v14-app examples.
nextjs-v14 is basically a direct copy of the existing v13 example just with the next version bumped. It still uses the pages router.
nextjs-v14-app was modified further from there to use the app router directory structure, using layout.tsx instead of _app.tsx, as well as moving the custom element and wrapped component behind the 'use client'; boundary. The Lit SSR patching does not work within React Server Components. This note is added to the example README as well as in the changelog entry up above.

Testing

Testing was done manually by running each example projects and inspecting the page source to confirm declarative shadow DOM was server rendered, there were no React hydration errors, and components became interactive.

Future work

It would be much better to have automated integration/e2e testing for these test nextjs projects.

While I updated the README for the @lit-labs/nextjs package, it would be good to have a fuller explanation of the caveats of app router support somewhere like on lit.dev's frameworks section.

Notes for releasing

The @lit-labs/ssr-react dependency minimum version should be updated for the next release of @lit-labs/nextjs. Both are getting "breaking" version bumps (minor for 0. package) so changesets might already do this but adding a note here as a reminder to check it.

changeset-bot · 2024-03-09T01:28:10Z

🦋 Changeset detected

Latest commit: f4bf397

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages

Name	Type
@lit-labs/ssr-react	Minor
@lit/react	Patch
@lit-labs/nextjs	Minor
@lit-internal/test-elements-react	Patch
@lit-examples/nextjs-v13	Patch
@lit-examples/nextjs-v14-app	Patch
@lit-examples/nextjs-v14	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2024-03-09T01:28:20Z

📊 Tachometer Benchmark Results

Summary

nop-update

this-change, tip-of-tree, previous-release: unsure 🔍 -4% - +6% (-0.40ms - +0.60ms)
^{this-change vs tip-of-tree}

render

this-change: 44.11ms - 46.32ms
this-change, tip-of-tree, previous-release: unsure 🔍 -5% - +3% (-0.94ms - +0.48ms)
^{this-change vs tip-of-tree}
this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +2% (-0.42ms - +0.80ms)
^{this-change vs tip-of-tree}
this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +1% (-0.72ms - +0.22ms)
^{this-change vs tip-of-tree}

update

this-change: 467.71ms - 480.90ms
this-change, tip-of-tree, previous-release: unsure 🔍 -6% - +5% (-2.46ms - +2.17ms)
^{this-change vs tip-of-tree}
this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +1% (-1.02ms - +0.86ms)
^{this-change vs tip-of-tree}
this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +1% (-3.64ms - +6.84ms)
^{this-change vs tip-of-tree}

update-reflect

this-change: 481.50ms - 496.85ms
this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +1% (-4.41ms - +5.18ms)
^{this-change vs tip-of-tree}

Results

this-change

Browser: chrome-headless 123.0.6312.122
Sample size: 50
Built by: Benchmarks #6215
Commit: f4bf397

render

Version	Avg time	vs
	44.11ms - 46.32ms	-

update

Version	Avg time	vs
	467.71ms - 480.90ms	-

update-reflect

Version	Avg time	vs
	481.50ms - 496.85ms	-

this-change, tip-of-tree, previous-release

Browser: chrome-headless 123.0.6312.122
Sample size: 30
Built by: Benchmarks #6215
Commit: f4bf397

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	17.62ms - 18.66ms	-	unsure 🔍 -5% - +3% -0.94ms - +0.48ms	unsure 🔍 -5% - +3% -0.84ms - +0.56ms
tip-of-tree tip-of-tree	17.89ms - 18.86ms	unsure 🔍 -3% - +5% -0.48ms - +0.94ms	-	unsure 🔍 -3% - +4% -0.58ms - +0.77ms
previous-release previous-release	17.81ms - 18.75ms	unsure 🔍 -3% - +5% -0.56ms - +0.84ms	unsure 🔍 -4% - +3% -0.77ms - +0.58ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	37.68ms - 41.05ms	-	unsure 🔍 -6% - +5% -2.46ms - +2.17ms	unsure 🔍 -5% - +7% -1.98ms - +2.71ms
tip-of-tree tip-of-tree	37.92ms - 41.10ms	unsure 🔍 -6% - +6% -2.17ms - +2.46ms	-	unsure 🔍 -5% - +7% -1.77ms - +2.78ms
previous-release previous-release	37.37ms - 40.63ms	unsure 🔍 -7% - +5% -2.71ms - +1.98ms	unsure 🔍 -7% - +4% -2.78ms - +1.77ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	10.57ms - 11.29ms	-	unsure 🔍 -4% - +6% -0.40ms - +0.60ms	unsure 🔍 -3% - +6% -0.36ms - +0.59ms
tip-of-tree tip-of-tree	10.47ms - 11.18ms	unsure 🔍 -6% - +4% -0.60ms - +0.40ms	-	unsure 🔍 -4% - +5% -0.46ms - +0.49ms
previous-release previous-release	10.49ms - 11.13ms	unsure 🔍 -5% - +3% -0.59ms - +0.36ms	unsure 🔍 -5% - +4% -0.49ms - +0.46ms	-

this-change, tip-of-tree, previous-release

Browser: chrome-headless 123.0.6312.122
Sample size: 50
Built by: Benchmarks #6215
Commit: f4bf397

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	32.81ms - 33.76ms	-	unsure 🔍 -1% - +2% -0.42ms - +0.80ms	unsure 🔍 -3% - +1% -0.86ms - +0.49ms
tip-of-tree tip-of-tree	32.72ms - 33.47ms	unsure 🔍 -2% - +1% -0.80ms - +0.42ms	-	unsure 🔍 -3% - +1% -0.99ms - +0.23ms
previous-release previous-release	33.00ms - 33.95ms	unsure 🔍 -1% - +3% -0.49ms - +0.86ms	unsure 🔍 -1% - +3% -0.23ms - +0.99ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	67.11ms - 68.44ms	-	unsure 🔍 -2% - +1% -1.02ms - +0.86ms	unsure 🔍 -1% - +2% -0.68ms - +1.04ms
tip-of-tree tip-of-tree	67.20ms - 68.52ms	unsure 🔍 -1% - +2% -0.86ms - +1.02ms	-	unsure 🔍 -1% - +2% -0.59ms - +1.12ms
previous-release previous-release	67.05ms - 68.14ms	unsure 🔍 -2% - +1% -1.04ms - +0.68ms	unsure 🔍 -2% - +1% -1.12ms - +0.59ms	-

this-change, tip-of-tree, previous-release

Browser: chrome-headless 123.0.6312.122
Sample size: 50
Built by: Benchmarks #6215
Commit: f4bf397

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	29.95ms - 30.51ms	-	unsure 🔍 -2% - +1% -0.72ms - +0.22ms	unsure 🔍 -2% - +1% -0.74ms - +0.23ms
tip-of-tree tip-of-tree	30.10ms - 30.85ms	unsure 🔍 -1% - +2% -0.22ms - +0.72ms	-	unsure 🔍 -2% - +2% -0.55ms - +0.54ms
previous-release previous-release	30.09ms - 30.88ms	unsure 🔍 -1% - +2% -0.23ms - +0.74ms	unsure 🔍 -2% - +2% -0.54ms - +0.55ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	482.85ms - 490.78ms	-	unsure 🔍 -1% - +1% -3.64ms - +6.84ms	unsure 🔍 -1% - +1% -3.63ms - +6.62ms
tip-of-tree tip-of-tree	481.78ms - 488.64ms	unsure 🔍 -1% - +1% -6.84ms - +3.64ms	-	unsure 🔍 -1% - +1% -4.83ms - +4.61ms
previous-release previous-release	482.07ms - 488.56ms	unsure 🔍 -1% - +1% -6.62ms - +3.63ms	unsure 🔍 -1% - +1% -4.61ms - +4.83ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	526.64ms - 532.77ms	-	unsure 🔍 -1% - +1% -4.41ms - +5.18ms	unsure 🔍 -1% - +1% -5.73ms - +4.11ms
tip-of-tree tip-of-tree	525.64ms - 533.00ms	unsure 🔍 -1% - +1% -5.18ms - +4.41ms	-	unsure 🔍 -1% - +1% -6.53ms - +4.13ms
previous-release previous-release	526.67ms - 534.37ms	unsure 🔍 -1% - +1% -4.11ms - +5.73ms	unsure 🔍 -1% - +1% -4.13ms - +6.53ms	-

_{tachometer-reporter-action v2 for Benchmarks}

github-actions · 2024-03-09T01:30:24Z

The size of lit-html.js and lit-core.min.js are as expected.

AndrewJakubowicz

Absolutely awesome work!

.changeset/polite-rice-stare.md

packages/labs/ssr-react/README.md

packages/labs/ssr-react/src/lib/node/wrap-create-element.ts

AndrewJakubowicz · 2024-03-11T03:12:37Z

packages/labs/ssr-react/src/lib/node/wrap-create-element.ts

+            // https://github.com/facebook/react/blob/v18.2.0/packages/react/src/ReactElement.js#L401-L417
+            if (children.length > 0) {
+              newChildren.push(...children);
+            } else if (Array.isArray(props?.children)) {


When is props.children an Array that hasn't be handled by the .length > 0 check above?

the params come like this

function createElement(type, props, ...children) {}

If children are provided as 3rd+ arguments, children.length > 0 is true and we use that.
Else, only 2 arguments were supplied, and there is a possibility that the 2nd arg props contains a children property that is meant to be the children. The value of that props.children could either be an array or non-array ReactNode if it's just a single item.

packages/labs/ssr-react/src/lib/node/wrap-create-element.ts

packages/labs/nextjs/src/index.ts

.changeset/chilled-plants-reflect.md

packages/react/src/create-component.ts

examples/nextjs-v14-app/README.md

justinfagnani

lgtm!

.changeset/polite-rice-stare.md

packages/labs/ssr-react/README.md

packages/labs/ssr-react/src/lib/node/wrap-jsx.ts

packages/labs/ssr-react/src/node/enable-lit-ssr.ts

- Use `collectResultSync()` - Add jsdoc to exported functions - `||` to `??` - Some doc wording changes

augustjk added 4 commits March 8, 2024 17:20

Add monkey-patching of runtime jsx

f10d7ee

Add nextjs v14 and app router support

9702f91

Use global flag for lit/react and labs/ssr-react prop coordination

d5dcabb

Create nextjs-v14 and app router examples

02a0c05

augustjk requested review from justinfagnani and AndrewJakubowicz March 9, 2024 01:28

augustjk requested a review from kevinpschaaf as a code owner March 9, 2024 01:28

augustjk linked an issue Mar 9, 2024 that may be closed by this pull request

[labs/nextjs] Support for the app directory #3657

Closed

1 task

AndrewJakubowicz reviewed Mar 11, 2024

View reviewed changes

augustjk added 4 commits March 11, 2024 14:45

readme and changset wording fixes

645ab6b

Remove wrapping function name check

031272a

Remove global check for jsx-dev-runtime

4a4e565

Merge branch 'main' into nextjs-support-v14

d7b5da5

keisha-rw mentioned this pull request Apr 3, 2024

[ssr] Investigate interactions with minified or unminified lit-html #4527

Open

justinfagnani approved these changes Apr 17, 2024

View reviewed changes

augustjk added 2 commits April 23, 2024 21:26

Address review comments

fe8d064

- Use `collectResultSync()` - Add jsdoc to exported functions - `||` to `??` - Some doc wording changes

Typo fix in changeset

f4bf397

augustjk merged commit aa4fc3e into main Apr 24, 2024
10 checks passed

augustjk deleted the nextjs-support-v14 branch April 24, 2024 05:55

lit-robot mentioned this pull request Apr 25, 2024

Version Packages #4632

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[labs/nextjs, labs/ssr-react, lit/react] Add support for Next.js v14 and App Router #4575

[labs/nextjs, labs/ssr-react, lit/react] Add support for Next.js v14 and App Router #4575

augustjk commented Mar 9, 2024 •

edited

changeset-bot bot commented Mar 9, 2024 •

edited

github-actions bot commented Mar 9, 2024 •

edited

📊 Tachometer Benchmark Results

Summary

nop-update

render

update

update-reflect

Results

render

update

update-reflect

render

update

nop-update

render

update

render

update

update-reflect

github-actions bot commented Mar 9, 2024

AndrewJakubowicz left a comment

AndrewJakubowicz Mar 11, 2024

augustjk Mar 11, 2024

justinfagnani left a comment

[labs/nextjs, labs/ssr-react, lit/react] Add support for Next.js v14 and App Router #4575

[labs/nextjs, labs/ssr-react, lit/react] Add support for Next.js v14 and App Router #4575

Conversation

augustjk commented Mar 9, 2024 • edited

Explanation of changes:

Testing

Future work

Notes for releasing

changeset-bot bot commented Mar 9, 2024 • edited

🦋 Changeset detected

github-actions bot commented Mar 9, 2024 • edited

📊 Tachometer Benchmark Results

Summary

nop-update

render

update

update-reflect

Results

render

update

update-reflect

render

update

nop-update

render

update

render

update

update-reflect

github-actions bot commented Mar 9, 2024

AndrewJakubowicz left a comment

Choose a reason for hiding this comment

AndrewJakubowicz Mar 11, 2024

Choose a reason for hiding this comment

augustjk Mar 11, 2024

Choose a reason for hiding this comment

justinfagnani left a comment

Choose a reason for hiding this comment

augustjk commented Mar 9, 2024 •

edited

changeset-bot bot commented Mar 9, 2024 •

edited

github-actions bot commented Mar 9, 2024 •

edited