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.

Sign up for GitHub

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

Jump to bottom

feat: save vitest state in global variable #739

Merged
merged 6 commits into from Feb 14, 2022
Merged

feat: save vitest state in global variable #739

merged 6 commits into from Feb 14, 2022

Conversation

poyoho
Copy link
Member

@poyoho poyoho commented Feb 12, 2022

fix: #663

save vitest state in global variable for the nice error message.

@netlify
Copy link

netlify bot commented Feb 12, 2022 •
edited

✔️ Deploy Preview for vitest-dev ready!

🔨 Explore the source changes: d4fb702

🔍 Inspect the deploy log: https://app.netlify.com/sites/vitest-dev/deploys/6207ab6960ef2c0008b1de5a

😎 Browse the preview: https://deploy-preview-739--vitest-dev.netlify.app

antfu
antfu reviewed Feb 12, 2022
View reviewed changes
packages/vitest/globals.d.ts Outdated
@@ -11,5 +11,6 @@ declare global {
const afterAll: typeof import('vitest')['afterAll']
const beforeEach: typeof import('vitest')['beforeEach']
const afterEach: typeof import('vitest')['afterEach']
let __vitest_worker__: import('vitest').WorkerGlobalState
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't expose this to user type. Let's have it in the source code which will be removed in build

@Demivan
Copy link
Member

Demivan commented Feb 12, 2022

Would be nice to have a test for this. Check that re-assigning process no longer breaks.

@poyoho
Copy link
Member Author

poyoho commented Feb 12, 2022 •
edited

TypeError: Cannot read properties of undefined (reading 'writableLength')

no, it will break, but with other error

because use the process in any space? and we cache the process in the global variable and make the user mock?

@poyoho
Copy link
Member Author

poyoho commented Feb 12, 2022 •
edited

@Demivan
may be can break the change (save the vitest state in the process.vitest)?
only cache the process in the global variable and reset it in every test file exec

@Demivan
Copy link
Member

Demivan commented Feb 12, 2022

I don't think 1538626 is needed. Users can still break it by re-assigning properties on process. So better error message is enough

@poyoho
Copy link
Member Author

poyoho commented Feb 12, 2022

ok!
but I don't know how re-assigning properties on process can break it. after run the test file always reset the process, and the test file are no preemption of process 🤔

@poyoho
Revert "test: mess process"
d4fb702 
This reverts commit 1538626.
@poyoho poyoho requested a review from antfu February 12, 2022 12:49
@antfu antfu merged commit 32ad454 into vitest-dev:main Feb 14, 2022
chaii3 pushed a commit to chaii3/vitest that referenced this pull request May 13, 2022
@webfansplz @antfu
feat(useScroll): new Function (vitest-dev#739)
bc6c342 
Co-authored-by: webfansplz <>
Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antfu antfu Awaiting requested review from antfu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Non declared variable causes vitest to freak out
3 participants
@poyoho @Demivan @antfu