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
feat: support npm setup files #1312
Conversation
✅ Deploy Preview for vitest-dev ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
packages/vitest/src/node/config.ts
Outdated
@@ -122,13 +123,15 @@ export function resolveConfig( | |||
if (process.env.VITEST_MIN_THREADS) | |||
resolved.minThreads = parseInt(process.env.VITEST_MIN_THREADS) | |||
|
|||
resolved.setupFiles = toArray(resolved.setupFiles || []).map(file => resolve(resolved.root, file)) | |||
const require = createRequire(resolve(resolved.root, '_')) |
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.
const require = createRequire(resolve(resolved.root, '_')) | |
const require = createRequire(resolve(resolved.root, '_')) |
What's the _
for?
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.
just a placeholder, createRequire
needs a file, maybe using index.js
more readable?
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.
Can we use resolveModule
from local-pkg
?
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.
I have tried, resolveModule
uses const _require = createRequire(import.meta.url)
, it's hard code as import.meta.url
, but we need relative to root
here. otherwise causes some bad cases.
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.
This will incorrectly resolve esm files. We use Vite's resolver, when importing files.
vitest/packages/vite-node/src/server.ts
Line 33 in 03a54e0
async resolveId(id: string, importer?: string): Promise<ViteNodeResolveId | null> { |
vitest/packages/vite-node/src/client.ts
Line 127 in 03a54e0
const resolveId = async (dep: string, callstackPosition = 1) => { |
Maybe we should just remove resolving for non-relative paths here. Reporter field for example supports these kind of pathes.
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.
vitest/packages/vitest/src/node/config.ts
Line 125 in 03a54e0
resolved.setupFiles = toArray(resolved.setupFiles || []).map(file => resolve(resolved.root, file)) |
The setupFiles
will prepend config.root
path directly,
{
test: {
setupFiles: ['@testing-library/jest-dom/extend-expect.js'],
},
}
will be transformed to:
{
test: {
setupFiles: ['/path/to/@testing-library/jest-dom/extend-expect.js'],
},
}
it should be:
{
test: {
setupFiles: ['/path/to/node_modules/@testing-library/jest-dom/extend-expect.js'],
},
}
LGTM 👍🏻 |
For example: