Skip to content
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

Remove custom optimizeDeps.include in react integration #12923

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Jan 7, 2025

Changes

fix #12896
ref #12735 (revert)

  • React: @vitejs/plugin-react will add optimizeDeps.include themselves
  • Vue: Added a comment of why we manually add vue to optimizeDeps.include
  • Solid: Different issue, but I noticed that we don't optimize @astrojs/solid-js/client.js by default like how the other integrations do.

I didn't do this for Preact as we're still pinning to an older version, but once we can update to the latest (tracked #12805), we can also do the same here for it.

Testing

Existing tests should pass

Docs

n/a. bug fix.

Copy link

changeset-bot bot commented Jan 7, 2025

🦋 Changeset detected

Latest commit: 5f05c9e

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

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 github-actions bot added pkg: vue Related to Vue (scope) pkg: react Related to React (scope) pkg: solid Related to Solid (scope) pkg: integration Related to any renderer integration (scope) labels Jan 7, 2025
@ascorbic
Copy link
Contributor

ascorbic commented Jan 9, 2025

Are we confident this won't break anything? If this is already handled by the Vite plugin I'm confused that the Sanity plugimns were having trouble

@bluwy
Copy link
Member Author

bluwy commented Jan 9, 2025

Apart from the react-compiler-runtime PR that fixes a Sanity issue, which this PR will likely break, it shouldn't break any other usecases. I'm happy to look into the Sanity issue if a repro can be shared by them, but at the meantime, I don't think unconditionally adding react-compiler-runtime to optimizeDeps.include is the right solution here.

@ascorbic
Copy link
Contributor

ascorbic commented Jan 9, 2025

@stipsan could you share a minimal repro so we can see?

@stipsan
Copy link
Contributor

stipsan commented Jan 9, 2025

@stipsan could you share a minimal repro so we can see?

Sure, I can find time by end of next week. Is it ok to hold off on reverting the fix until then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: integration Related to any renderer integration (scope) pkg: react Related to React (scope) pkg: solid Related to Solid (scope) pkg: vue Related to Vue (scope)
Projects
None yet
3 participants