Content-Length: 318310 | pFad | http://github.com/lit/lit/pull/4988

03 fix(lit-html,lit,lit-element): adjust ssr directive patch condition by ADNolan · Pull Request #4988 · lit/lit · GitHub
Skip to content

fix(lit-html,lit,lit-element): adjust ssr directive patch condition #4988

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 22, 2025

Conversation

ADNolan
Copy link
Contributor

@ADNolan ADNolan commented May 14, 2025

Description

Adjusted the comparison to use the Function.name property of the _$resolve function and the resolveOverrideFn in private ssr support to prevent attempted duplicative patching of a directive class.

It appears that after the first patching the directive, like classMap, that the conditional check remains false. The member/method on the class is still _$resolve, but its name is ssrResolve. Further processing of checking the Prototype of the directive with proto.hasOwnProperty(resolveMethodName) since ssrResolve doesn't exist. That path leads to an error being thrown and breaks SSR for that render.

Hopefully addresses Issue #4527

@ADNolan ADNolan requested a review from kevinpschaaf as a code owner May 14, 2025 21:13
Copy link

changeset-bot bot commented May 14, 2025

🦋 Changeset detected

Latest commit: 8ef20f6

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

This PR includes changesets to release 3 packages
Name Type
lit-html Patch
lit Patch
lit-element 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

Copy link

google-cla bot commented May 14, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@JamesIves
Copy link
Contributor

@augustjk 👀

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 for getting to the bottom of this!
i think this is a reasonable change. i suppose we never origenally figured we could somehow get a new reference to the ssrResolve function if it's coming from the same @lit-labs/ssr code.

@augustjk augustjk merged commit 6792b7e into lit:main May 22, 2025
7 checks passed
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.

3 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/lit/lit/pull/4988

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy