Skip to content

Resolve peer dependency problem in the integration packages (SolidJS, Vue, Svelte, React)#12481

Merged
ematipico merged 9 commits into
withastro:mainfrom
marbrex:main
Nov 21, 2024
Merged

Resolve peer dependency problem in the integration packages (SolidJS, Vue, Svelte, React)#12481
ematipico merged 9 commits into
withastro:mainfrom
marbrex:main

Conversation

@marbrex
Copy link
Copy Markdown
Contributor

@marbrex marbrex commented Nov 20, 2024

Changes

Moved vite from devDependencies to the dependencies of @astrojs/solid-js in its package.json.

Before

Yarn in PnP mode produces the following error :

failed to import "@astrojs/solid-js"
|- Error: vite-plugin-solid tried to access vite (a peer dependency) but it isn't provided by your application; this makes the require call ambiguous and unsound.

Required package: vite (via "vite/package.json")
Required by: vite-plugin-solid

After

The problem is resolved. Astro works fine.
Closes #12460.

Description

This PR addresses an issue (#12460) where @astrojs/solid-js does not declare vite as a dependency in its package.json. As a result, users encounter a peer dependency error when using this integration, particularly because vite-plugin-solid relies on vite but cannot find it unless explicitly provided by the consuming project.

While astro itself includes vite as a dependency, peer dependencies cannot rely on transitive dependencies. This causes issues when configuring the SolidJS integration in the astro.config.js, as @astrojs/solid-js is unable to resolve vite.

By adding vite as a dependency of @astrojs/solid-js, this PR ensures :

  1. The peer dependency for vite in vite-plugin-solid is resolved properly.
  2. Users no longer need to manually add vite to their project's dependencies or use workarounds like Yarn's packageExtensions.

Testing

Tested locally with Yarn's packageExtenstions.

Docs

Not sure if docs should be updated or not.
/cc @withastro/maintainers-docs for feedback!

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Nov 20, 2024

🦋 Changeset detected

Latest commit: d8a7f5d

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: solid Related to Solid (scope) pkg: integration Related to any renderer integration (scope) labels Nov 20, 2024
@ematipico
Copy link
Copy Markdown
Member

What if we moved vite inside the peerDependencies? It feels like a better fix to me 🤔

@marbrex
Copy link
Copy Markdown
Contributor Author

marbrex commented Nov 20, 2024

@ematipico It could be another solution, but in this case it would mean that users will still need to manually install vite in their project to satisfy the peer dependency, which can lead to confusion if they aren't familiar with managing peer dependencies. This could reintroduce the original issue.

@ematipico
Copy link
Copy Markdown
Member

but in this case it would mean that users will still need to manually install vite in their project to satisfy the peer dependency

In this case, no, because Astro installs vite as a direct dependency, so the users wouldn't need to do anything. Plus, other integrations that rely on vite plugins such as @astrojs/svelte and @astrojs/vue don't do anything fancy around vite.

I believe we are hitting a specific issue of Yarn 🤔

@marbrex
Copy link
Copy Markdown
Contributor Author

marbrex commented Nov 20, 2024

@ematipico Indeed, it seems to be related to the the way Yarn PnP operates. It is possible that the vite instance inside astro's dependency tree is scoped to astro and cannot satisfy a peer dependency declared by another package, @astrojs/solid-js in this case.

@bluwy
Copy link
Copy Markdown
Member

bluwy commented Nov 20, 2024

Yeah if it's a peer dep, the user has to install vite themselves. It can't access Astro's vite dep.

This issue is a little tricky because it's not only affecting solid. svelte, vue, etc are also affected. It worked with pnpm because it hoists packages by default (more like private hoisting than a public one, which is what npm is doing).

I guess the right thing to do given what we have is to put vite in dependency like in this PR, but I think we should do this for solid, svelte, vue, react, preact all at once.

@marbrex
Copy link
Copy Markdown
Contributor Author

marbrex commented Nov 20, 2024

@bluwy Yes, I think so as well. I'm happy to adjust the PR accordingly! 😊

@github-actions github-actions Bot added pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope) pkg: react Related to React (scope) labels Nov 20, 2024
@marbrex marbrex changed the title Resolve peer dependency problem in the SolidJS integration package Resolve peer dependency problem in the integration packages (SolidJS, Vue, Svelte, React) Nov 20, 2024
Comment thread .changeset/angry-parrots-push.md Outdated
@github-actions github-actions Bot added the pkg: preact Related to Preact (scope) label Nov 20, 2024
Comment thread .changeset/strong-stingrays-provide.md Outdated
Copy link
Copy Markdown
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

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

I think the only concern with this is potential Vite version mismatches, but hopefully the package manager is smart enough to dedupe it.

Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com>
@marbrex
Copy link
Copy Markdown
Contributor Author

marbrex commented Nov 21, 2024

@bluwy @ematipico Seems like one of the test jobs timed out. The time has exceeded 25 minutes.

@ematipico ematipico merged commit 8a46e80 into withastro:main Nov 21, 2024
@astrobot-houston astrobot-houston mentioned this pull request Nov 21, 2024
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: preact Related to Preact (scope) pkg: react Related to React (scope) pkg: solid Related to Solid (scope) pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integration "@astrojs/solid-js" does not provide "vite" to "vite-plugin-solid"

3 participants