Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(build): add additional metadata for mapping to improve map paths #2935

Merged
merged 1 commit into from
Jul 25, 2024

Conversation

castastrophe
Copy link
Collaborator

@castastrophe castastrophe commented Jul 25, 2024

Description

After adding @spectrum-css/typography imports to Storybook, we started seeing:

Error: An error occurred while trying to read the map file at components/typography/index.css.map
Error: ENOENT: no such file or directory, open '/Users/carobert/Projects/spectrum/spectrum-css/components/typography/dist/components/typography/index.css.map'

in the console which reflects an invalid path being used in the sourcemap files generated by the component-builder. This fix improves the path generated and removes the error messaging from our Storybook build.

How and where has this been tested?

Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.

Validation steps

  1. Do a clean local build (storybook symlinks to local dist); yarn build.
  2. Kick off the storybook server: yarn start
  • Expect to see no logging in the output related to invalid local *.css maps.

To-do list

Copy link

changeset-bot bot commented Jul 25, 2024

⚠️ No Changeset found

Latest commit: 79e67f4

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

@castastrophe castastrophe self-assigned this Jul 25, 2024
@castastrophe castastrophe added size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. build Issues associated with the build process; often a refactor skip_vrt Add to a PR to skip running VRT (but still pass the action) labels Jul 25, 2024
Copy link
Contributor

github-actions bot commented Jul 25, 2024

🚀 Deployed on https://pr-2935--spectrum-css.netlify.app

Copy link
Contributor

github-actions bot commented Jul 25, 2024

File metrics

Summary

Total size: 4.63 MB*

🎉 No changes detected in any packages

* Size determined by adding together the size of the main file for all packages in the library.
* Results are not gzipped or minified.
* An ASCII character in UTF-8 is 8 bits or 1 byte.

Copy link
Collaborator

@marissahuysentruyt marissahuysentruyt left a comment

Choose a reason for hiding this comment

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

Great work! This resolve the source map errors you mentioned. 🎉

I don't think this is on our end, but I do see these warnings as well (on main and this branch):

Sourcemap for "/virtual:/@storybook/builder-vite/setup-addons.js" points to missing source files
Sourcemap for "/virtual:/@storybook/builder-vite/vite-app.js" points to missing source files

Is this anything we'd need to address in the future? Either way, I don't think they are blocking this from merging, so approved!

@castastrophe
Copy link
Collaborator Author

Great work! This resolve the source map errors you mentioned. 🎉

I don't think this is on our end, but I do see these warnings as well (on main and this branch):

Sourcemap for "/virtual:/@storybook/builder-vite/setup-addons.js" points to missing source files
Sourcemap for "/virtual:/@storybook/builder-vite/vite-app.js" points to missing source files

Is this anything we'd need to address in the future? Either way, I don't think they are blocking this from merging, so approved!

Yes! I looked into that earlier and found this is a known bug: storybookjs/storybook#28567. It doesn't impact the rendering so I thought we were okay to stay at this version of Vite instead of rolling back.

@castastrophe castastrophe enabled auto-merge (squash) July 25, 2024 17:32
@castastrophe castastrophe merged commit 7f0de63 into main Jul 25, 2024
12 checks passed
@castastrophe castastrophe deleted the chore-improve-css-map-paths branch July 25, 2024 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues associated with the build process; often a refactor ready-for-review size-1 XS ~1-6hrs; nearly trivial, a few hours, could do more than one in a single day. skip_vrt Add to a PR to skip running VRT (but still pass the action)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants