Skip to content

fix(react-runtime): temporary Vitest/jsdom compatibility shim for forwarded props#793

Open
mhoritani wants to merge 1 commit into
stenciljs:mainfrom
mhoritani:fix/react-vitest-jsdom-compat-shim
Open

fix(react-runtime): temporary Vitest/jsdom compatibility shim for forwarded props#793
mhoritani wants to merge 1 commit into
stenciljs:mainfrom
mhoritani:fix/react-vitest-jsdom-compat-shim

Conversation

@mhoritani
Copy link
Copy Markdown

@mhoritani mhoritani commented Apr 27, 2026

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally for affected output targets
  • Tests (npm test) were run locally and passed
  • Prettier (npm run prettier) was run locally and passed

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue URL: #791

In Vitest + jsdom, wrappers produced by @stencil/react-output-target can lose forwarded element props like id.

The root of it is export resolution: @lit/react has separate browser and node entries, and Vitest's transform context can pick the node entry. That node path is SSR-oriented and does not run the browser prop-application effect, so props classified as element props never land on the custom element in tests.

What is the new behavior?

  • Adds jsdom regression coverage for native prop forwarding (id) and custom element prototype property forwarding (object value path).
  • Replaces runtime delegation in packages/react/src/runtime/create-component.ts with a temporary compatibility shim aligned with @lit/react browser create-component behavior.
  • Keeps the public runtime API unchanged and leaves SSR runtime behavior untouched.

This is intentionally framed as a temporary compatibility layer, not a move away from Lit.

Does this introduce a breaking change?

  • Yes
  • No

Other information

  • I also validated with external reproductions tied to bug: React output target runtime resolves to node/SSR path under Vitest and drops custom element prop forwarding #791, including a no-workaround run for the standalone repro.
  • Temporary shim exit criteria are documented in the code comments: once @lit/react provides a stable way to preserve browser-style prop forwarding semantics in this Vitest/jsdom scenario, the shim should be removed and direct delegation can return.
  • My read is that the long-term fix likely belongs upstream in @lit/react. I did not want to block users on an uncertain upstream timeline, so this PR takes a short-term compatibility path.
  • @johnjenkins: does this look like the right short-term direction here, or would you prefer a different approach in this repo while upstream is unresolved?

@mhoritani mhoritani requested a review from a team as a code owner April 27, 2026 19:49
@johnjenkins
Copy link
Copy Markdown
Contributor

hey @mhoritani - thanks for looking at this, but it feels like the correct place for you to raise this would be in @lit/react ?

  1. I'm reticent to accept a large change like this just for a testing scenario.
  2. it's recreating core @lit/react stuff in this lib and just using @lit/react for types - feels very fragile and not maintainable

@mhoritani
Copy link
Copy Markdown
Author

mhoritani commented Apr 28, 2026

Upstream PR is now open in Lit: lit/lit#5325

Given feedback here, I’m treating that as the primary fix path and can follow up in output-targets based on the outcome upstream.

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.

2 participants