You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Since the regex change introduced in the above PR, certain source code with a huge inline source map leads to freeze main vitest process for expensive regexpHoistable.test(code) quite a long time. (See comment in #4855 (comment))
I'm not really familiar with estimating regex perf, but the arbitrary whitespace prefix [ \t]* seems to be bad and it's probably redundant since \b already asserts word boundary.
To reproduce this, build vitest itself with pnpm dev (which adds inline sourcemap), then run:
pnpm -C test/browser test-fixtures run
(... it gets stuck for a while since expensive regex test takes over main thread. waiting for a while to see...)
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Vitest caught 1 unhandled error during the test run.
This might cause false positive tests. Resolve unhandled errors to make sure your tests are not affected.
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Error ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Error: [birpc] timeout on calling "getFiles"
❯ ../../../../../../../../../__vitest__/assets/index-0EYobN63.js:5:60901
This error originated in"basic.test.ts"test file. It doesn't mean the error was thrown inside the file itself, but while it was running.⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
It's also odd that Vitest's own file such as packages/vitest/dist/vendor/vi.smj1Ggd4.js goes through these transform, but I suppose that's a different issue with externalization during local dev.
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
Ideally, include a test that fails without this PR but passes with it.
Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.
Tests
Run the tests with pnpm test:ci.
Documentation
If you introduce new functionality, document it. You can run documentation with pnpm run docs command.
Changesets
Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.
It's also odd that Vitest's own file such as packages/vitest/dist/vendor/vi.smj1Ggd4.js goes through these transform, but I suppose that's a different issue with externalization during local dev.
All imported files go through a transformation when running in the browser. I am not sure what is odd here. Vitest is not bundled by the optimizer on purpose.
All imported files go through a transformation when running in the browser. I am not sure what is odd here. Vitest is not bundled by the optimizer on purpose.
Yeah, you're right. I thought seeing many files vitest/dist/vendor/... are odd, but now I get that those files are genuinely imported from vitest/utils, browser, runners etc.... I'm still digesting how browser mode works. Thanks for clarifying it!
I forgot to mention but regexpHoistable.test(code) is still false for all internal dependencies and vite/dist chunks, so it's not costing unnecessary parsing/magic-string work.
hi-ogawa
changed the title
fix(vitest): simplify hoist transform check regex to avoid regex DoS
fix(vitest): simplify hoist transform check regex to avoid expensive regex match
Jan 16, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
await vi.hoisted#4962Since the regex change introduced in the above PR, certain source code with a huge inline source map leads to freeze main vitest process for expensive
regexpHoistable.test(code)quite a long time. (See comment in #4855 (comment))I'm not really familiar with estimating regex perf, but the arbitrary whitespace prefix
[ \t]*seems to be bad and it's probably redundant since\balready asserts word boundary.To reproduce this, build vitest itself with
pnpm dev(which adds inline sourcemap), then run:It's also odd that Vitest's own file such as
packages/vitest/dist/vendor/vi.smj1Ggd4.jsgoes through these transform, but I suppose that's a different issue with externalization during local dev.Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yamlunless you introduce a new test example.Tests
pnpm test:ci.Documentation
pnpm run docscommand.Changesets
feat:,fix:,perf:,docs:, orchore:.