Skip to content

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
@changeset-bot
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

@google-cla
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 originally figured we could somehow get a new reference to the ssrResolve function if it's coming from the same @lit-labs/ssr code.

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