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

Improve sourcemaps #437

Merged
merged 1 commit into from Jul 13, 2022
Merged

Improve sourcemaps #437

merged 1 commit into from Jul 13, 2022

Conversation

IanVS
Copy link
Member

@IanVS IanVS commented Jul 13, 2022

Turns out we weren't creating good sourcemaps, especially in dev. It seems that whereas rollup only uses the mappings, I guess that vite/esbuild uses the sources field of the sourcemap as well, and without passing the source option to MagicString's generateMap, we were losing the connection. So, this adds the id as the source, and also enables hires (high resolution) so that columns are correct and not just lines (useful for code coverage, for example).

I also added MagicString to some of our other plugins which weren't using it.

The most obvious changes are in the react and svelte projects. Vue is a bit improved, but we're still losing the story file sourcemaps for some reason.

To test, fire up an example project (in dev especially, but good to check prod builds too), look in the sources tab in devtools, and poke around, looking for sourcemapped versions of the files. You should see them in this branch, but not nearly as many, if any, in main.

@yannbf, I also tested these changes by copying the dist folder from this project into the node_modules of your https://github.com/yannbf/storybook-coverage-recipes project, and got good results from the react_vite and svelte_vite examples there.

@IanVS IanVS requested a review from joshwooding July 13, 2022 04:15
@yannbf
Copy link
Member

yannbf commented Jul 13, 2022

This is incredible. Thank you so much @IanVS!


const namedExportsOrder = Object.entries<any>(stories)
.filter(([, def]) => !def.template)
.map(([id]) => id);

const output = [
codeWithoutDefaultExport,
'',
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could just remove this right?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it's needed so that a newline is added.

@IanVS IanVS merged commit 2330d3c into main Jul 13, 2022
@IanVS IanVS deleted the sourcemaps branch July 13, 2022 13:05
IanVS added a commit that referenced this pull request Jul 13, 2022
Turns out we weren't creating good sourcemaps, especially in dev.  It seems that whereas rollup only uses the `mappings`, I guess that vite/esbuild uses the `sources` field of the sourcemap as well, and without passing the `source` option to MagicString's `generateMap`, we were losing the connection.  So, this adds the `id` as the source, and also enables `hires` (high resolution) so that columns are correct and not just lines (useful for code coverage, for example).

I also added MagicString to some of our other plugins which weren't using it.

The most obvious changes are in the react and svelte projects.  Vue is a bit improved, but we're still losing the story file sourcemaps for some reason.

To test, fire up an example project (in dev especially, but good to check prod builds too), look in the sources tab in devtools, and poke around, looking for sourcemapped versions of the files.  You should see them in this branch, but not nearly as many, if any, in main.
@IanVS
Copy link
Member Author

IanVS commented Jul 13, 2022

Released in v0.1.41.

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.

None yet

3 participants