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

Build: bundle csf-tools with tsup #19141

Merged
merged 5 commits into from
Sep 26, 2022
Merged

Build: bundle csf-tools with tsup #19141

merged 5 commits into from
Sep 26, 2022

Conversation

IanVS
Copy link
Member

@IanVS IanVS commented Sep 8, 2022

Issue:

Refs: #18732

What I did

Converted @storybook/csf-tools to use tsup bundling script. Also removed core-js since it's no longer needed.

How to test

CI

@IanVS IanVS added maintenance User-facing maintenance tasks core labels Sep 8, 2022
@IanVS
Copy link
Member Author

IanVS commented Sep 8, 2022

@ndelangen, any idea why the unit tests of core-server might be failing here? They use csf-tools, but I can't figure out why the bundling change would have broken them.

@IanVS IanVS requested a review from ndelangen September 8, 2022 11:53
@ndelangen
Copy link
Member

I took a look at this and something is breaking:

 FAIL  lib/core-server/src/utils/stories-json.test.ts
  useStoriesJson
    JSON endpoint
      ✓ scans and extracts index (23 ms)
      ✓ scans and extracts stories v3 (11 ms)
      ✓ scans and extracts stories v2 (9 ms)
      ✕ disallows .mdx files without storyStoreV7 (13 ms)
      ✓ allows disabling storyStoreV7 if no .mdx files are used (9 ms)
      ✓ can handle simultaneous access (7 ms)
    SSE endpoint
      ✕ sends invalidate events (11 ms)
      ✕ only sends one invalidation when multiple event listeners are listening (4 ms)
      ✕ debounces invalidation events (16 ms)
  convertToIndexV3
    ✓ converts v7 index.json to v6 stories.json (1 ms)

  ● useStoriesJson › JSON endpoint › disallows .mdx files without storyStoreV7

    expect(received).toEqual(expected) // deep equality

    - Expected  - 1
    + Received  + 3

    - You cannot use `.mdx` files without using `storyStoreV7`.
    + CSF: missing default export /Users/me/Projects/Storybook/core/code/lib/core-server/src/utils/__mockdata__/src/nested/Page.stories.mdx (line 1, col 0)
    +
    + More info: https://storybook.js.org/docs/react/writing-stories/introduction#default-export

      193 |
      194 |       expect(send).toHaveBeenCalledTimes(1);
    > 195 |       expect(send.mock.calls[0][0]).toEqual(
          |                                     ^
      196 |         'You cannot use `.mdx` files without using `storyStoreV7`.'
      197 |       );
      198 |     });

      at Object.<anonymous> (lib/core-server/src/utils/stories-json.test.ts:195:37)

  ● useStoriesJson › SSE endpoint › sends invalidate events

    expect(jest.fn()).toHaveBeenCalledTimes(expected)

    Expected number of calls: 1
    Received number of calls: 0

      280 |
      281 |       await onChange('src/nested/Button.stories.ts');
    > 282 |       expect(mockServerChannel.emit).toHaveBeenCalledTimes(1);
          |                                      ^
      283 |       expect(mockServerChannel.emit).toHaveBeenCalledWith(STORY_INDEX_INVALIDATED);
      284 |     });
      285 |

      at Object.<anonymous> (lib/core-server/src/utils/stories-json.test.ts:282:38)

  ● useStoriesJson › SSE endpoint › only sends one invalidation when multiple event listeners are listening

    O: CSF: missing default export /Users/me/Projects/Storybook/core/code/lib/core-server/src/utils/__mockdata__/src/nested/Page.stories.mdx (line 1, col 0)

    More info: https://storybook.js.org/docs/react/writing-stories/introduction#default-export

      11 |
      12 |               More info: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#story-store-v7
    > 13 |             `)}},ImportDeclaration:{enter({node:r}){let{source:e}=r;if(o.isStringLiteral(e))t.imports.push(e.value);else throw new Error("CSF: unexpected import source")}}}),!t._meta)throw new O(t._ast,t._fileName);if(!t._meta.title&&!t._meta.component)throw new Error(b.dedent`
         |                                                                                                                                                                                              ^
      14 |         CSF: missing title/component ${h(t._ast,t._fileName)}
      15 |
      16 |         More info: https://storybook.js.org/docs/react/writing-stories/introduction#default-export

      at j.parse (lib/csf-tools/dist/index.js:13:190)
      at Object.csfIndexer [as indexer] (lib/core-server/src/utils/stories-json.test.ts:52:58)
      at StoryIndexGenerator.extractStories (lib/core-server/src/utils/StoryIndexGenerator.ts:212:19)
      at lib/core-server/src/utils/StoryIndexGenerator.ts:133:35
          at async Promise.all (index 5)
          at async Promise.all (index 1)
      at StoryIndexGenerator.updateExtracted (lib/core-server/src/utils/StoryIndexGenerator.ts:127:5)
      at StoryIndexGenerator.ensureExtracted (lib/core-server/src/utils/StoryIndexGenerator.ts:149:5)
      at StoryIndexGenerator.initialize (lib/core-server/src/utils/StoryIndexGenerator.ts:113:5)
      at getInitializedStoryIndexGenerator (lib/core-server/src/utils/stories-json.test.ts:72:3)
      at lib/core-server/src/utils/stories-json.ts:49:25
      at Object.<anonymous> (lib/core-server/src/utils/stories-json.test.ts:270:7)

  ● useStoriesJson › SSE endpoint › debounces invalidation events

The critical bit is:

CSF: missing default export /Users/me/Projects/Storybook/core/code/lib/core-server/src/utils/mockdata/src/nested/Page.stories.mdx (line 1, col 0)

I removed the JSON.parse for a bit and this is what the snapshot was updated to:

      expect(send.mock.calls[0][0]).toMatchInlineSnapshot(`
        "CSF: missing default export /Users/me/Projects/Storybook/core/code/lib/core-server/src/utils/__mockdata__/src/nested/Page.stories.mdx (line 1, col 0)

        More info: https://storybook.js.org/docs/react/writing-stories/introduction#default-export"
      `);

So my conclusion is: the mdx transformer is not operable after the change.
@shilman might be able to help?

What's interesting is that when I run: yarn nx run @storybook/csf-tools:prep --reset --optimized it's broken, but if I remove the --optimized flag and run this instead:

yarn nx run @storybook/csf-tools:prep --reset

The tests seem to pass locally..
Somehow the minification breaks the runtime, OOF 👎

# Conflicts:
#	code/lib/csf-tools/package.json
The name can be minified, it's safer to check if the error is the correct instance.
@IanVS
Copy link
Member Author

IanVS commented Sep 21, 2022

@ndelangen I found the issue. We were using err.name, which isn't reliable to use when minifying, unless keep-names https://esbuild.github.io/api/#keep-names is enabled.

The remaining CI failures look unrelated.

@ndelangen ndelangen merged commit b725ff9 into next Sep 26, 2022
@ndelangen ndelangen deleted the tsup/csf-tools branch September 26, 2022 13:35
@ndelangen
Copy link
Member

@IanVS OW AWESOME!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants