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

fix: use actual vite version in version check #5586

Merged
merged 2 commits into from
Jul 18, 2022

Conversation

dominikg
Copy link
Member

to allow custom resolutions, local links etc

the previous implementation did a read of local devDependencies. That does not work when pnpm overrides are used, or the local devDependency is set to a link of a local version etc.

Vite exports it's version so just use that instead.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented Jul 18, 2022

🦋 Changeset detected

Latest commit: 9e7650a

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

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit 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

@Rich-Harris
Copy link
Member

Ha, I was going to do this initially but I was worried about this...

project/
├ node_modules/
│ ├ @sveltejs/
│ │ ├ kit/
│ │ │ ├ node_modules/
│ │ │ │ └ vite # @3
│ └ vite # @2.9

...completely forgetting that it's a peer dependency instead of a normal one

@@ -2,7 +2,7 @@ import fs from 'fs';
import path from 'path';
import colors from 'kleur';
import { svelte } from '@sveltejs/vite-plugin-svelte';
import { searchForWorkspaceRoot } from 'vite';
import { searchForWorkspaceRoot, version as vite_version } from 'vite';
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this works in Vite 2 though, version is only exported in Vite 3. Maybe we should check vite/package.json instead.

Copy link
Member

Choose a reason for hiding this comment

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

oh, whoops. presumably that means mucking about with import-meta-resolve which is a bit of a pain. what if we import * as vite from 'vite' and then check for the existence of the version export? if it's there, check it, if not, assume it's version 2

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