Skip to content

Conversation

@valoriecarli
Copy link
Contributor

@valoriecarli valoriecarli commented Aug 28, 2025

This PR revisits introducing vitest into our stack.
Please see relevent comments in the applicable files.
Refactored original PoC to align with updated dependencies & monorepo changes since initial spike.

References:

Overall status: The Vitest runner is allegedly faster than Jest. Where the actual tests do run faster, the overall process takes almost 2x as long. This exercise is to:
1.) Find out why
2.) How is Jest handling the task that takes longer for Vitest.
3.) How to sidestep this issue.

Of importance - Isolation must be true. Though it does not impact this particular repo, it's mandatory for mc-fe, so the idea here is to keep that variable constant.


Notes: Stats taken from 5a48e9e (includes vmThreads & file optimization)

Known - Collection time (specifically tied to presets) determined to be the most resource intensive.

jest - Time ~18s
vitest: Duration 10.69s (transform 6.65s, setup 506ms, collect 80.99s, tests 1.97s, environment 256ms, prepare 9.25s)

Collection is an aggregate CPU time across all threads. From this example, with 10 CPUs, this is about ~8.1s of work per CPU (~81% of the total work).

What happens during the collection phase:

  • Discover test files
  • Reading file contents
  • Resolving imports & aliases --> Tested & excluded from being the underlying cause.
  • Building dependency graphs --> This is thought to be the culprit.

Why are dependency graphs thought to be the culprit?

  • These map how all your modules connect & creates a network of dependencies that vitest has to process.
  • 810 preset files all import from @/core, creating a massive dependency explosion.
  • Test file → Preset file → @/core → @jackfranklin/test-data-bot → @faker-js/faker + lodash
  • Vitest has to:
    • Follow every import path
    • Parse every file
    • Build the entire dependency tree (see below for a single example).
    • Process heavy modules like @faker-js/faker

Where the current commit stands (how we got from ~27s to ~10s):

1.) Excludes preset implementation files

  • **/presets/**/*.ts - Excludes all .ts files in preset directories.
  • This prevents vitest from processing the 810+ preset files during the collect phase.
  • Avoids the heavy dependency chain (preset.ts → @/core → @jackfranklin/test-data-bot → @faker-js/faker)

2.) Re-includes preset spec files

  • This allows preset spec files to still run & be tested.
  • Spec files can still import presets when they actually run the tests.
    (Note that by default, vitest includes/excludes certain files. Once you start to customize it though, it overrides those).

Why is this working so far? Now during the collection phase:

  • Vitest skips processing 810+ preset files, dramatically reducing the dependency graph.
  • During test execution spec files can still import & test presets on-demand.
    TL;DR - Functionality is preserved: All tests still run, but the collect phase is much faster. The preset files don't need to be processed during the collection phase - they only need to be available when the spec files actually run the tests.

Eg Before updated config when vitest processes all files:

  • empty.spec.ts (test file)
  • empty.ts (preset file) ← Resource hog
  • full.spec.ts (test file)
  • full.ts (preset file) ← Resource hog

Eg After updated config when vitest processes only spec files:

  • empty.spec.ts (test file)
  • empty.ts (preset file) ← skipped during collection phase
  • full.spec.ts (test file)
  • full.ts (preset file) ← skipped during collection phase

A visual example of a single dependency graph (whomever takes this over should check it out, its pretty hypnotic):
Screenshot 2025-09-19 at 12 29 13

What is Jest doing that it handles this differently? read but not confirmed.

@valoriecarli valoriecarli self-assigned this Aug 28, 2025
@valoriecarli valoriecarli added 🚧 Status: WIP fe-chapter-rotation Tasks coming from frontend chapter work labels Aug 28, 2025
@changeset-bot
Copy link

changeset-bot bot commented Aug 28, 2025

⚠️ No Changeset found

Latest commit: 5a48e9e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@valoriecarli valoriecarli changed the title test(vitest): spike no2 FEC-259: Vitest POC Aug 29, 2025
Comment on lines 34 to 47
resolve: {
alias: {
'@/core': path.resolve(__dirname, './standalone/src/core'),
'@/core/test-utils': path.resolve(
__dirname,
'./standalone/src/core/test-utils'
),
'@/graphql-types': path.resolve(
__dirname,
'./standalone/src/graphql-types'
),
'@/models': path.resolve(__dirname, './standalone/src/models'),
'@/utils': path.resolve(__dirname, './standalone/src/utils'),
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If comparing to the original PoC, the release of Vitest 3.x became a little more strict about path resolution & now requires explicit aliases.

Comment on lines +71 to +74
"devDependencies": {
"@vitest/coverage-v8": "^3.2.4",
"@vitest/ui": "^3.2.4",
"vitest": "^3.2.4"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if comparing to the original PoC, devDependency [email protected] was causing several issues and determined to be outdated/incompatible at this point. No longer needed.

Comment on lines +1 to +7
// Add custom stringFromArray matcher
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(expect as any).stringFromArray = (allowed: string[]) => ({
asymmetricMatch: (value: unknown) =>
typeof value === 'string' && allowed.includes(value),
getExpectedType: () => 'string',
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As noted here, the original PoC relied on a now outdated dependency. This is the workaround..for now.

Comment on lines +22 to +28
targets: expect.arrayContaining([
expect.objectContaining({
addressKey: expect.any(String),
quantity: expect.any(Number),
shippingMethodKey: null,
}),
]),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Full disclosure, I do not know why this wasn't needed in the original PoC, or if vitest just became a little more strict since then to require us to explicitly define the expected structure.

@valoriecarli valoriecarli marked this pull request as ready for review August 29, 2025 18:10
@valoriecarli valoriecarli requested review from a team as code owners August 29, 2025 18:10
@valoriecarli valoriecarli requested review from a team and removed request for a team August 29, 2025 18:10
@valoriecarli valoriecarli changed the title FEC-259: Vitest POC FEC-259: Vitest PoC #2 Aug 29, 2025
@CarlosCortizasCT CarlosCortizasCT marked this pull request as draft September 2, 2025 14:52
},
maxConcurrency: os.cpus().length,
// vmthreads for better isolation
pool: 'vmThreads',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

vmThreads reads as though it slows down the run, but it actually increases it in our case.
unclear if this is a viable option
Improving Vitest performance

@CarlosCortizasCT
Copy link
Contributor

Based on the learnings of this PoC we decided to not migrate to vitest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fe-chapter-rotation Tasks coming from frontend chapter work ⛔ do not merge 🚧 Status: WIP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants