-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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(coverage): c8 aliased paths #3306
Conversation
Run & review this pull request in StackBlitz Codeflow. |
const [moduleUrl] = await this.resolveUrl(modulePath) | ||
const href = pathToFileURL(moduleUrl).href |
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.
When an aliased modules is loaded the fsPath
doesn't contain extension at all. @sheremet-va
should I make this change to fsPath
instead so that everywhere it's used would be affected, e.g. stacktraces I think?
With alias, without PR changes:
import { addThreeNumbers } from "src/logic/utils";
{
__filename: '/x/y/repro/src/logic/utils',
id: 'src/logic/utils',
fsPath: 'src/logic/utils'
}
Without alias, without PR changes, works OK:
import { addThreeNumbers } from "../../logic/utils";
{
__filename: '/x/y/repro/src/logic/utils.ts',
id: '/src/logic/utils.ts',
fsPath: '/x/y/repro/src/logic/utils.ts'
}
With alias, with PR changes:
import { addThreeNumbers } from "src/logic/utils";
{
__filename: '/x/y/repro/src/logic/utils.ts',
id: 'src/logic/utils',
fsPath: 'src/logic/utils'
}
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 should not even be allowed. Vitest should fail if the module is not resolved. I am coming up with a fix for this. Some people rely on tsconfig resolution when in practice we don't support 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.
I see, the reproduction case is not using alias
in vitest.config.ts
. By adding following the coverage works OK:
alias: {
src: fileURLToPath(new URL("./src", import.meta.url)),
},
So I guess in your fix Vitest will fail to load module (or warn?) if only tsconfig.json
has alias
but vitest.config.ts
does not?
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.
We can finally stop supporting this weird edge case: #3307
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.
So I guess in your fix Vitest will fail to load module (or warn?) if only tsconfig.json has alias but vitest.config.ts does not?
People rely on the fact that their modules are relative to the baseDir
which is never true in Vite. Because of this, we have a lot of issues when the file is not resolved correctly, but it can still fetch module code.
vitenode.fetchCache
contains file path without extension when module is imported using alias. This does not end up in coverage report due to missing extension.