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

fix: sourcemap generation for story files #11

Conversation

valentinpalkovic
Copy link
Contributor

@valentinpalkovic valentinpalkovic commented May 24, 2024

Related Counter PR at Storybook: storybookjs/storybook#27171

Currently, the sourcemap generation for story files is broken. If rspack behaves like Webpack, then loaders have to chain sourcemaps. This doesn't happen automatically, as Vite/Rollup does. The issue is that the magic-string doesn't support sourcemap chaining currently.

The files I have touched only add content at the end of the file. Therefore, creating source maps is not essential since the line/column mapping will stay the same.

This fix only ensures that the source view with enabled sourcemaps in the browser isn't completely broken for .stories.ts|js files. The source map mapping still doesn't refer to the original file, but some compiled file returned by some loader in between. I haven't spent too much time to figure out the issue altogether. Someone else could continue from here. The linked PR in Storybook fixes source maps completely for Vite and Webpack5 builders in Storybook

Reproduction

  1. Activate sourcemaps in your browser's developer console
  2. Change the Button.stories.tsx in a freshly generated Storybook to
import type { Meta, StoryObj } from "@storybook/react";
import { fn } from "@storybook/test";
import { Button } from "./Button";

// More on how to set up stories at: https://storybook.js.org/docs/writing-stories#default-export
const meta = {
  title: "Example/Button",
  component: Button,
  args: {
    onClick: () => {
      throw new Error("Not implemented");
    },
  },
} satisfies Meta<typeof Button>;

export default meta;
type Story = StoryObj<typeof meta>;

// More on writing stories with args: https://storybook.js.org/docs/writing-stories/args
export const Primary: Story = {
  args: {
    primary: true,
    label: "Button",
  },
};
  1. Click on a button and try to visit the file in your browser, which throws the error
  2. Before the fix, the browser couldn't open the source view at all
  3. At least the source view works, although the mapping still does not refer to the original source, but to some precompiled code in between. It seems, that some rspack loaders still don't generate/chain sourcemaps correctly.

@valentinpalkovic valentinpalkovic changed the title Fix partially sourcemaps Fix sourcemaps partially May 24, 2024
@valentinpalkovic valentinpalkovic changed the title Fix sourcemaps partially fix: sourcemap generation for story files May 24, 2024
@valentinpalkovic valentinpalkovic force-pushed the valentin/fix-source-maps-for-story-files branch from 8030992 to 4f5e1c0 Compare May 24, 2024 11:21
@valentinpalkovic valentinpalkovic force-pushed the valentin/fix-source-maps-for-story-files branch from 4f5e1c0 to 1fd7584 Compare May 24, 2024 11:25
@fi3ework
Copy link
Member

Confirmed this could resume a basic sourcemap from an error. I'll raise an issue to track the uncompleted sourcemap. Thanks for the sync up from storybook's main repository! 🙌♥️

@fi3ework fi3ework merged commit 4938dbf into rspack-contrib:main May 26, 2024
1 check passed
@fi3ework fi3ework added the sync Sync up changes from storybookjs/storybook label May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sync Sync up changes from storybookjs/storybook
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants