Skip to content

Switch build system to vite and update jest#32

Merged
daniel-heppner-ibigroup merged 29 commits intostorybook-9from
storybook-9-vite-build
Aug 6, 2025
Merged

Switch build system to vite and update jest#32
daniel-heppner-ibigroup merged 29 commits intostorybook-9from
storybook-9-vite-build

Conversation

@daniel-heppner-ibigroup
Copy link
Copy Markdown

@daniel-heppner-ibigroup daniel-heppner-ibigroup commented Jul 10, 2025

  • Replace webpack config with vite config in package.json and the storybook config. Directly translated the webpack config to vite config.
  • Removed things that didn't seem necessary anymore; mainly the ufuzzy override (it seems to work without), and the getAboslutePath() thing. Not sure why we even had that.
  • Replaced require() with import across the project since vite is esm only.
  • Removed some references to /lib/ in various imports around the code. In some cases it was drilling in to a package to extract a type/property that wasn't exported, so I had to add it to the exports from the package's index file. This mostly had to do with core utils.
  • Updated jest to the latest version. It shaves a few seconds off the run time. This required some minor config changes to make work, mainly the graphql loader. We had an 8 year old package to import graphql, but that was unneeded as a file loader works fine.
  • Snapshots updated. Jest formats the snapshots slightly differently now days, as it prints the objects using a normal javascript serialization instead of printing the prototypes for each object. ("Array", "Object", etc) This can be reverted with a config change but I thought it was fine. Looks like the styled component classnames changed for some reason. Not sure why.
  • Reorganized the order of operations in the github CI a bit. This is mainly so that we don't have to wait for playwright to install before tests that don't need it run, so it can fail faster.
  • Removed typecheck from the prepublish command. We were already running this on its own in CI so it was really unnecessary.
  • Fixes a weird issue (bug?) in the new storybook test runner where the test serializer has to export its functions individually rather than as part of a default export. I will have to report this to them because their example shows the old method. This fixed the snapshot tests.

Comment thread .storybook/main.js
}
}

function getAbsolutePath(value) {
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I removed this function. I'm not sure what its original function was, but it doesn't seem necessary anymore.

*/
function isRunningJest() {
return process.env.JEST_WORKER_ID !== undefined;
}
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

this hasn't done anything for a long time since the snapshot tests don't run in jest since I did storybook 7 update

@daniel-heppner-ibigroup daniel-heppner-ibigroup changed the title Storybook 9 vite build Update storybook to 9, switch build system to vite. Jul 11, 2025
@daniel-heppner-ibigroup daniel-heppner-ibigroup changed the title Update storybook to 9, switch build system to vite. Switch build system to vite and update jest Jul 11, 2025
Copy link
Copy Markdown

@miles-grant-ibigroup miles-grant-ibigroup left a comment

Choose a reason for hiding this comment

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

A few things I'm worried about. But it does work well

Comment thread .github/workflows/node-ci.yml
Comment thread package.json
Comment thread package.json Outdated
Comment thread package.json Outdated
</path>
</svg>
<span class="Alert__AlertHeader-sc-2vlo3n-2 fpauiN">
<span class="sc-eqUAAy cHzsWi">
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We make use of the Alert__AlertHeader type classnames in some places. Is there any way to restore these?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

are you sure about that? If you look that classname contained a random styled component ID already- it wasn't just Alert__AlertHeader; it had -sc-2vlo3n-2 on there so it's not like you could use it as a selector anywhere that you don't have access to a regular expression. (like in CSS)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I actually don't know why this changed. I guess it has something to do with the styled components plugin for vite. Since this classname is dynamically generated by the build step there's no guarantee that it will be consistent. For example, in OTP-RR these classnames are generated by the vite/webpack configuration in otp-rr, unrelated to the vite config that generated this snapshot. So relying on it is not safe because it depends on the consumer of the component.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Here's an example of a selector we currently use: svg[class*="styled__FromIcon"] This needs to continue to work

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I figured it out and adjusted the config. However, it doesn't matter since the classnames in OTP-RR are generated by the config over there and this only affects the storybook output.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The unit tests don't seem to have changed? What's going on

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

wdym? The snapshots were updated in 0c3d19a.

Comment thread packages/trip-details/src/trip-detail.tsx
Copy link
Copy Markdown

@miles-grant-ibigroup miles-grant-ibigroup left a comment

Choose a reason for hiding this comment

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

I'm nervous, but things seem to be working (and much quicker!) So let's give it a go and worst case scenario we can revert

Copy link
Copy Markdown

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

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

Dev mode starts much faster!

Comment thread .github/workflows/node-ci.yml Outdated
import {
getLegRouteLongName,
getLegRouteShortName
} from "@opentripplanner/core-utils/lib/itinerary";
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These changes away from importing from lib are nice.

Co-authored-by: Binh Dam <56846598+binh-dam-ibigroup@users.noreply.github.com>
@daniel-heppner-ibigroup
Copy link
Copy Markdown
Author

Before I merge this, please review the parent PR! Once it's merged into that it'll be harder to keep the changes separate.

@daniel-heppner-ibigroup daniel-heppner-ibigroup merged commit 4586c44 into storybook-9 Aug 6, 2025
2 checks passed
@daniel-heppner-ibigroup daniel-heppner-ibigroup deleted the storybook-9-vite-build branch August 6, 2025 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants