chore(js): Remove misleading top-level Vitest configuration #20234

SyntaxColoring · 2025-11-24T20:08:37Z

Overview

This is a simplification to our JS toolchain configuration.

Details

For what I suspect are accidental/historical reasons, our Vite and Vitest config has been working like this:

flowchart BT
    global_vite["top-level vite.config.mts"]
    global_vitest["top-level vitest.config.mts"]

    global_vitest -- "imports and merges with" --> global_vite

    app
    protocol-designer
    components
    etc["etc..."]

    app
    app -- "main config" --> app_vite["app/vite.config.mts"]
    app -- "test config" --> global_vitest

    protocol-designer
    protocol-designer -- "main config" --> protocol_designer_vite["protocol-designer/vite.config.mts"]
    protocol-designer -- "test config" --> global_vitest

    components
    components -- "test config" --> global_vitest
    components -- "main config" --> components_vite["components/vite.config.mts"]

    storybook -- "main config" --> global_vite

    etc["etc..."]

Each project had its main Vite configuration in a local vite.config.mts file.

We also had a global vite.config.mts file. Confusingly, it wasn't involved in any project's configuration. It was used by Storybook, and, indirectly, by Vitest.

Changelog

This PR doesn't fully resolve the mismatch between how Vitest+Storybook are configured globally, whereas the project themselves are configured locally. (In my opinion, we ought to go fully local, but I haven't achieved that here.)

What this PR does do is do away with the global vite.config.mts file. For Vitest, it's replaced by vitest.config.mts, which already existed; stuff was essentially split up arbitrarily between the two. For Storybook, it's replaced by a new .storybook/vite.config.mts. I think these are improvements because they make it clearer what the files actually affect.

So we end up with:

flowchart BT
    global_vitest["top-level vitest.config.mts"]

    app
    protocol-designer
    components
    etc["etc..."]

    app
    app -- "main config" --> app_vite["app/vite.config.mts"]
    app -- "test config" --> global_vitest

    protocol-designer
    protocol-designer -- "main config" --> protocol_designer_vite["protocol-designer/vite.config.mts"]
    protocol-designer -- "test config" --> global_vitest

    components
    components -- "test config" --> global_vitest
    components -- "main config" --> components_vite["components/vite.config.mts"]

    storybook -- "main config" --> storybook_vite[".storybook/vite.config.mts"]

    etc["etc..."]

Test Plan and Hands on Testing

Tests all continue to pass
Smoke-check some of the JS projects' make dev commands
- make -C app dev
- make -C components dev
- make -C protocol-designer dev
Smoke-check some of the JS projects' sandbox builds
- https://sandbox.components.opentrons.com/what_happens_if_we_delete_the_test_viteconfigs/
- https://sandbox.designer.opentrons.com/what_happens_if_we_delete_the_test_viteconfigs/

Review requests

None in particular. I guess just, does this direction for simplification sound good? Incrementally simplify this for now, and eventually move everything to project-local config files?

Risk assessment

Medium. Things will totally break (and not be caught by tests) if I've gotten this wrong and any of the JS projects are relying on the global vite.config.mts file.

vitest.config.mts was its only consumer.

mjhuff

Thanks for the writeup. This makes sense to me with the test plan, thank you!

codecov · 2025-11-24T21:30:04Z

Codecov Report

❌ Patch coverage is 12.50000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 24.28%. Comparing base (d14703b) to head (d1d6f94).

Files with missing lines	Patch %	Lines
.storybook/main.ts	12.50%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             edge   #20234      +/-   ##
==========================================
- Coverage   26.02%   24.28%   -1.74%     
==========================================
  Files        3627     3621       -6     
  Lines      301690   301119     -571     
  Branches    42198    42126      -72     
==========================================
- Hits        78500    73114    -5386     
- Misses     223168   227985    +4817     
+ Partials       22       20       -2

Flag	Coverage Δ
protocol-designer	`19.24% <12.50%> (+<0.01%)`	⬆️
step-generation	`5.59% <12.50%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
.storybook/main.ts	`3.57% <12.50%> (+3.57%)`	⬆️

... and 119 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

SyntaxColoring · 2025-11-24T22:05:31Z

~~Ughhh, I think Storybook was relying on the old top-level vite.config.ts somehow. I'll look into it.~~ PR description updated with changes.

Resolve conflicts in: * storybook/main.ts * vitest.config.mts

SyntaxColoring · 2025-11-25T16:04:06Z

.storybook/main.ts

-
-  async viteFinal(config) {
-    config.resolve = config.resolve || {}
-    config.resolve.alias = config.resolve.alias || {}
-
-    // Add the same alias as in app/vite.config.mts to support /app/ imports
-    config.resolve.alias['/app/'] = path.resolve(__dirname, '../app/src/') + '/'
-
-    return config
-  },


I think this viteFinal block, introduced in #20228, was not doing anything. vite.config.mts (now moved to .storybook/vite.config.mts) already had this /app/ alias. I tested this removal by looking at some components that were importing things from /app.

FYI @koji

SyntaxColoring · 2025-11-25T16:06:07Z

.storybook/main.ts

+  // todo(mm, 2025-11-25): This is meant to allow loading images in e.g.
+  // DeckFixtureSetupInstructionsModal, but it doesn't seem to be working.
  staticDirs: ['../app/src/assets'],


@koji Heads up for this, too. From #20228 I'm guessing this was meant to fix image loading in DeckFixtureSetupInstructionsModal, but I get a 404 loading the image with or without this line. edge has the same problem so I don't think it's related to this PR's vite.config.mts changes.

Ohh wait, it's working in the sandbox link but it's not working with make -C components dev. That's weird.

mjhuff

Cool, happy with where this ended up all things considered. The eventual goal of project-local config files sounds good to me.

…lete_the_test_viteconfigs Resolve conflicts in: * .github/workflows/ll-test-build-deploy.yaml * .github/workflows/pd-test-build-deploy.yaml

Merge top-level vite.config.mts with vitest.config.mts.

d324e11

vitest.config.mts was its only consumer.

SyntaxColoring requested review from a team, jerader and smb2268 and removed request for a team and smb2268 November 24, 2025 20:38

SyntaxColoring marked this pull request as ready for review November 24, 2025 20:41

mjhuff approved these changes Nov 24, 2025

View reviewed changes

Add Vitest config to JS test workflows.

70c1d4d

Type-safe Storybook config.

57d5f27

SyntaxColoring marked this pull request as draft November 24, 2025 22:05

SyntaxColoring added 5 commits November 24, 2025 17:44

Preserve the old top-level vite.config.mjs for Storybook.

e691fe4

Merge branch 'edge' into what_happens_if_we_delete_the_test_viteconfigs

485bbe2

Resolve conflicts in: * storybook/main.ts * vitest.config.mts

Delete viteFinal alias that was doing the same thing as vite.config.mts.

1bd0574

Fix staticDirs?

69ae9ae

Leave a todo comment for staticDirs.

e3ce391

SyntaxColoring changed the title ~~chore(js): Consolidate Vitest configuration~~ chore(js): Remove misleading top-level Vitest configuration Nov 25, 2025

SyntaxColoring commented Nov 25, 2025

View reviewed changes

SyntaxColoring marked this pull request as ready for review November 25, 2025 16:06

SyntaxColoring requested review from koji and mjhuff November 25, 2025 16:06

mjhuff approved these changes Nov 25, 2025

View reviewed changes

Merge remote-tracking branch 'origin/edge' into what_happens_if_we_de…

d1d6f94

…lete_the_test_viteconfigs Resolve conflicts in: * .github/workflows/ll-test-build-deploy.yaml * .github/workflows/pd-test-build-deploy.yaml

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore(js): Remove misleading top-level Vitest configuration #20234

chore(js): Remove misleading top-level Vitest configuration #20234

Uh oh!

SyntaxColoring commented Nov 24, 2025 •

edited

Loading

Uh oh!

mjhuff left a comment

Uh oh!

codecov bot commented Nov 24, 2025 •

edited

Loading

Uh oh!

SyntaxColoring commented Nov 24, 2025 •

edited

Loading

Uh oh!

SyntaxColoring Nov 25, 2025 •

edited

Loading

Uh oh!

SyntaxColoring Nov 25, 2025

Uh oh!

SyntaxColoring Nov 25, 2025 •

edited

Loading

Uh oh!

mjhuff left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chore(js): Remove misleading top-level Vitest configuration #20234

Are you sure you want to change the base?

chore(js): Remove misleading top-level Vitest configuration #20234

Uh oh!

Conversation

SyntaxColoring commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Details

Changelog

Test Plan and Hands on Testing

Review requests

Risk assessment

Uh oh!

mjhuff left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

SyntaxColoring commented Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SyntaxColoring Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SyntaxColoring Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

SyntaxColoring Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjhuff left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SyntaxColoring commented Nov 24, 2025 •

edited

Loading

codecov bot commented Nov 24, 2025 •

edited

Loading

SyntaxColoring commented Nov 24, 2025 •

edited

Loading

SyntaxColoring Nov 25, 2025 •

edited

Loading

SyntaxColoring Nov 25, 2025 •

edited

Loading