-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(sveltekit): add Vite peer dep for proper type resolution #12926
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @mcous thanks a lot for opening this PR!
I had no idea that pnpm would pick the wrong version. I guess that's another entry in the book of pnpm's weird module resolution behaviours 😅
The change looks fine to me. In our docs, we recommend users to use Vite 4.2 or newer. However, we phrase it in a way that it's not strictly required. So for now I vote we continue with *
as a version to fix.
I just started CI. Once everything is green, I'll merge this PR.
(heads-up: Just assigning myself to this PR to track internally that I'm reviewing it)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests (including SvelteKit e2e tests) passed, so let's merge this. Thanks for contributing @mcous!
Before submitting a pull request, please take a look at our
Contributing guidelines and verify:
yarn lint
) & (yarn test
).Hello! I am in the process of setting up the SvelteKit plugin within a monorepo that includes multiple applications with multiple versions of Vite. I've found that because
@sentry/sveltekit
does not specify a peer dependency onvite
,pnpm
picks up the wrongvite
version for the package'simport type { Plugin } from 'vite'
statement, causing type errors in my Vite config.This PR adds a simple, optional peer dependency on Vite to
@sentry/sveltekit
to ensure that it picks up whatever version of Vite is being used in the specific package it's installed in, without auto-installingvite
or anything disruptive like that. Let me know if you'd prefer specific versions spec'd rather than*
, or if you'd prefer to not mark the dependency asoptional: true
Note
I set up Volta as instructed in the Contributing guide and used the spec'd versions of Node and yarn. None of
yarn build
,yarn lint
, noryarn test
fully passed on my machine on a fresh copy of thedevelop
branch