Skip to content

fix(@lit-labs/ssr-react,@lit-labs/nextjs): Enable ssr create element patch#4993

Merged
augustjk merged 2 commits intolit:mainfrom
ADNolan:enable-ssr-create-element-patch
Jun 24, 2025
Merged

fix(@lit-labs/ssr-react,@lit-labs/nextjs): Enable ssr create element patch#4993
augustjk merged 2 commits intolit:mainfrom
ADNolan:enable-ssr-create-element-patch

Conversation

@ADNolan
Copy link
Contributor

@ADNolan ADNolan commented May 20, 2025

Description

Added a guard to check if React.createElement was already patched with litPatchedCreateElement to prevent adding to the stack multiple calls to litPatchedCreateElement.

Addresses issue #4986

@ADNolan ADNolan requested a review from kevinpschaaf as a code owner May 20, 2025 15:05
@changeset-bot
Copy link

changeset-bot bot commented May 20, 2025

🦋 Changeset detected

Latest commit: eafea45

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

This PR includes changesets to release 2 packages
Name Type
@lit-labs/ssr-react Patch
@lit-labs/nextjs 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

@ADNolan
Copy link
Contributor Author

ADNolan commented May 20, 2025

@augustjk just a ping since its React SSR related and we have had some comments back and forth on the issues.

Object.assign((React as any).default ?? React, {
createElement: wrapCreateElement(React.createElement),
});
if (React.createElement.name !== 'litPatchedCreateElement') {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't litPatchedCreateElement be minified? I think it'd be safer to add a well-known property to the function in wrap-create-element.ts and check for that here. The property should be use computed property syntax to protect against minifiers and compilers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is code meant to run in the server, though I suppose it's not impossible that server code could be minified.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have confirmation this fixes the issue for others, I'm happy to merge this as is. We can address it then if minification becomes an issue.

@digitalsadhu
Copy link

I have this issue and when I manually applied the fix proposed in this PR into my local node_modules, it solved the issue for me. 👍

Copy link
Member

@augustjk augustjk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Object.assign((React as any).default ?? React, {
createElement: wrapCreateElement(React.createElement),
});
if (React.createElement.name !== 'litPatchedCreateElement') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have confirmation this fixes the issue for others, I'm happy to merge this as is. We can address it then if minification becomes an issue.

@augustjk augustjk merged commit a33d679 into lit:main Jun 24, 2025
7 checks passed
@raoufswe
Copy link
Contributor

raoufswe commented Jul 9, 2025

Hi @augustjk @ADNolan
Thanks so much for looking into this and getting it done 🙌
I'm keen to test the fix across a few different Next.js applications. Would it be possible to cut a release with the changes?

@lit-robot lit-robot mentioned this pull request Jul 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants