fix: use Vite root config setting instead of current working directory#15469
fix: use Vite root config setting instead of current working directory#15469
root config setting instead of current working directory#15469Conversation
🦋 Changeset detectedLatest commit: 4728f85 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
| */ | ||
| export function get_page_options(filepath) { | ||
| export function get_page_options(filepath, root) { | ||
| const input = read(path.resolve(root, filepath)); |
There was a problem hiding this comment.
Moved this out of the try...catch to avoid silent errors for files it can't find when calling read. The real issue was that the route filepaths weren't being resolved correctly. That's not an issue anymore now that we resolve against the root.
| id: '/blog.json', | ||
| pattern: '/^/blog.json/?$/', | ||
| endpoint: { file: 'samples/basic/blog.json/+server.js', page_options: null } | ||
| endpoint: { file: 'samples/basic/blog.json/+server.js', page_options: {} } |
There was a problem hiding this comment.
This fixes the test results to match the change from https://github.com/sveltejs/kit/pull/15469/changes#diff-8701fabc771d319105f7401163b7bc28189c5a11e30706215a62fb303385797a . Previously, the page options were incorrectly computed as null (unable to statically analyse). However, the correct value is {} (which means no page options exported).
|
|
||
| fs.writeFileSync( | ||
| './src/version.js', | ||
| path.join(import.meta.dirname, '..', 'src', 'version.js'), |
There was a problem hiding this comment.
Helps the version generation test run from the Vitest extension by avoiding resolving to the wrong path
| const process = await import('node:process'); | ||
|
|
||
| relative = (file) => path.relative(process.cwd(), file); | ||
| relative = (file) => path.relative('.', file); |
There was a problem hiding this comment.
Apparently '.' is equivalent to process.cwd() so that helps simplify things here
| it('should be the exact version from package.json', async () => { | ||
| const { VERSION } = await import(join(import.meta.dirname, 'version.js')); | ||
| const pkg = JSON.parse( | ||
| readFileSync(fileURLToPath(new URL('../package.json', import.meta.url)), 'utf-8') | ||
| ); | ||
| assert.equal( | ||
| VERSION, | ||
| pkg.version, | ||
| 'VERSION export in src/version.js does not equal version in package.json' | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Had to put the test logic inside of it so Vitest would stop skipping it from the IDE extension
|
|
||
| typescript: { | ||
| config: (config) => { | ||
| config.include.push('../unit-test'); |
There was a problem hiding this comment.
Avoids a typescript error from displaying in the unit test file
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
benmccann
left a comment
benmccann
left a comment
There was a problem hiding this comment.
needs a changeset, but besides that it looks good to me. will this result in any changes that users of vite can see or only vitest? it might be good to call this a feat:/minor so that it's more prominent in the changelog as I imagine it will be a semi-impactful change for a subset of users
Co-authored-by: Tee Ming <chewteeming01@gmail.com>
3 participants
closes #11356
closes #12140
closes #15188
This PR standardises the use of
process.cwd()to prefer the Vite configrootsetting when available similar to VPS. This helps us resolve paths correctly before reading files or generating tsconfig paths. It eases lots of monorepo tooling usage, particularly Vitest workspaces. This also allows us to use the Vitest IDE extension to run unit tests (which didn't work before because of the aforementioned issues).We could backport these changes to v2 but it's a bit easier to base it on v3 which already makes use of
import.meta.dirnamein many places thanks to #15434I've only made changes to
kititself for now, so other packages still default toprocess.cwd()but those are probably less problematic since they won't run for Vitest testsPlease don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits