Switch Storybook build to vite builder #745
Merged
Conversation
This file contains 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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #745 +/- ##
==========================================
- Coverage 79.57% 79.44% -0.13%
==========================================
Files 237 238 +1
Lines 4964 4973 +9
Branches 1350 1355 +5
==========================================
+ Hits 3950 3951 +1
- Misses 983 992 +9
+ Partials 31 30 -1 ☔ View full report in Codecov by Sentry. |
sergei-maertens
force-pushed
the
chore/724-sb-to-vitejs
branch
from
5fc4dcd to
2943dec
Compare
sergei-maertens
changed the title
sergei-maertens
force-pushed
the
chore/724-sb-to-vitejs
branch
from
2943dec to
b1e989a
Compare
sergei-maertens
force-pushed
the
chore/724-sb-to-vitejs
branch
from
81a271c to
0d654bb
Compare
sergei-maertens
force-pushed
the
chore/724-sb-to-vitejs
branch
from
0d654bb to
70052f8
Compare
We can detect the environment based on the presence of 'process' or not, and adapt the reading of environment variables based on the detected environment. This means that we currently have to duplicate the configuration, but as long as this is a temporary measure, we should be fine.
require doesn't exist in Vite, but import.meta can be used instead.
And silence the TS error about guard, it's fine at runtime.
Having looked into various configurations and plugins to automatically get the CommonJS transformed into ESM breaks either dev mode or production builds of Storybook. There are some external plugins for CommonJS transformations, but they either looks sketchy or don't seem to work in obvious ways (in particular @rollup/plugin-commonjs wasn't a success). So I've opted for the simple solution - write a custom plugin that just replaces the CJS export with an ESM export and it works in both dev and prod modes without any additional fuss. In the future, we will process other formats of these token exports, for which the design-token-editor package needs to be upgraded.
Vite copies the contents of public/\* as is into dist, which conflicts with the storybook static build. Furthermore, for how we use and distribute the SDK, the index.html and sdk.html are not relevant anyway, they're only used during local development with CRA. The base change from / to ./ should make things work better when deployed/hosted on an arbitrary subpath.
We cannot yet move the fonts to the public dir or change the SCSS that loads the fonts, as that would break the actual CRA-based build which now correctly ships the fonts. Because of the relative URLs used in the font-face declaration, Vite expects the fonts to exist in the root of the project.
The formio styles include an invalid at-rule in the middle of the file, which vite-css correctly warns about. The workaround updates the SCSS to use @use rather than @import to ensure the CSS is included rather than referenced, and this in turn makes it compatible with the sass preprocessor options to disable the charset at-rule, effectively getting rid of the warning.
CRA seems to properly translate the relative path and include the files while Vite requires some extra hints to properly point to the package location before it can find the font files. Unfortunately, both setups are not compatible, so in the vite config we override the sass-variable to pass down to the sass module import, where the default is set to the current, legacy behaviour.
Since we use the Vite builder instead.
The preVisit hook is flaky, especially with local development it's bad, but other people seem to be running into issues too (see https://github.com/storybookjs/test-runner/issues/442\). Let's see if this setup still has viewport support in Chromatic, which is what we care about most for snapshotting.
lodash/template requires the window._ global to be defined, otherwise the templates don't render properly. Failing to provide this breaks the formio file upload stories.
When running tests using @storybook/test-runner, we need to apply the same viewport to get the proper responsive elements as we get in the real browser/storybook. However, we just ripped out the viewport support because it's super flaky. So, we disable these tests here, but they should still run on Chromatic so we still get feedback there (provided we don't disable the Chromatic snapshotting).
sergei-maertens
force-pushed
the
chore/724-sb-to-vitejs
branch
from
70052f8 to
d613ea7
Compare
Vite/Rollup seem to be much more aggressive in handling imports with side-effects, resulting in the Dutch locale not getting registered on the flatpickr instance. Changing the import to a named import and manually assigning it fixes the issues in the static storybook build, while not breaking the CRA build (verified locally by creating a production build and using that with the real backend).
This is due to a difference between webpack and Vite/Rollup - CRA/wp is configured to not mangle class names in the minification process, while Vite/Rollup do mangle them. One option is to disable class name mangling which would probably only have a minor effect on the bundle sizes, however it's just technically more correct to point to the class itself rather than the name of the class in the runtime time, as this will be more robust for future build tooling changes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Partly closes #724
Migrate Storybook to ViteJS
To figure out still
/design-tokens/dist/tokens.jsrequireimports