-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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(snapshot): provide config
to resolveSnapshotPath
#6800
feat(snapshot): provide config
to resolveSnapshotPath
#6800
Conversation
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -573,7 +574,11 @@ export interface InlineConfig { | |||
/** | |||
* Resolve custom snapshot path | |||
*/ | |||
resolveSnapshotPath?: (path: string, extension: string) => string | |||
resolveSnapshotPath?: ( |
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 also expose a type from vitest/node
? Maybe ResolveSnapshotPathHandler
?
Can we keep a single scope? Several scopes just make it harder to parse release notes. "vitest" is kind of redundant. The scope is not about the package |
config
to resolveSnapshotPath
config
to resolveSnapshotPath
Description
Currently call flow looks like this:
Context is technically not snapshot package level concern, but in order to pass it through, I changed
SnapshotManager
. Probably there is a different way to achieve something similar without changingSnapshotManager
, but I think this works for now. Bigger structural change would be required later anyways for #1620.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yaml
unless you introduce a new test example.Tests
pnpm test:ci
.Documentation
pnpm run docs
command.Changesets
feat:
,fix:
,perf:
,docs:
, orchore:
.