Skip to content

fix(babel-preset-storybook-full-source): use named exports for story detection#32267

Merged
dmytrokirpa merged 3 commits into
microsoft:masterfrom
dmytrokirpa:fix_docsite_positioning_stories
Aug 13, 2024
Merged

fix(babel-preset-storybook-full-source): use named exports for story detection#32267
dmytrokirpa merged 3 commits into
microsoft:masterfrom
dmytrokirpa:fix_docsite_positioning_stories

Conversation

@dmytrokirpa

@dmytrokirpa dmytrokirpa commented Aug 9, 2024

Copy link
Copy Markdown
Contributor

Previous Behavior

After the SB7 migration, a regression occurred where the Positioning Components page is not functioning correctly. This issue arises because the PositionedComponent is defined prior to the story component, leading the babel-full-source-plugin to mistakenly treat it as a story component.

image

New Behavior

The babel-plugin-full-source detects stories as React components that are exported using named exports.

Related Issue(s)

@fabricteam

fabricteam commented Aug 9, 2024

Copy link
Copy Markdown
Collaborator

📊 Bundle size report

✅ No changes found

@dmytrokirpa
dmytrokirpa marked this pull request as ready for review August 9, 2024 09:42
@dmytrokirpa
dmytrokirpa requested review from a team as code owners August 9, 2024 09:42

@Hotell Hotell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IMO the proposed solution won't scale and tries to assume things that will not be clear to follow.

Instead we should update the AST traversal within the plugin to process only storybook components, which AFAIR are only those that are exported ( with optionally containing storybook specific static property assigments ) and live in *.stories.tsx? files.

thoughts?

@dmytrokirpa

dmytrokirpa commented Aug 9, 2024

Copy link
Copy Markdown
Contributor Author

IMO the proposed solution won't scale and tries to assume things that will not be clear to follow.

Instead we should update the AST traversal within the plugin to process only storybook components, which AFAIR are only those that are exported ( with optionally containing storybook specific static property assigments ) and live in *.stories.tsx? files.

thoughts?

The suggestion will work if we'd use babel plugin before any other transformers like SWC. In our case we use SWC to transform TS, JSX and imports, and babel plugin transforms source without those.

@dmytrokirpa
dmytrokirpa requested a review from Hotell August 12, 2024 06:39
@Hotell

Hotell commented Aug 12, 2024

Copy link
Copy Markdown
Contributor

The suggestion will work if we'd use babel plugin before any other transformers like SWC. In our case we use SWC to transform TS, JSX and imports, and babel plugin transforms source without those.

hmm not sure I follow ?

  • AFAIR we needed to switch to use unshift in order to make our babel-plugin work during migration
  • why would we need TS,JSX and imports ? swc will produce ES6 output ( target: 'es2019',) so exports will be there following the same syntax

@dmytrokirpa
dmytrokirpa force-pushed the fix_docsite_positioning_stories branch from 4009323 to b99b7b5 Compare August 12, 2024 11:00
@dmytrokirpa
dmytrokirpa removed the request for review from a team August 12, 2024 11:03
@dmytrokirpa

Copy link
Copy Markdown
Contributor Author

AFAIR we needed to switch to use unshift in order to make our babel-plugin work during migration
That doesn't help with the babel's inputSourceMap string issue

why would we need TS,JSX and imports ? swc will produce ES6 output ( target: 'es2019',) so exports will be there following the same syntax

That's actually correct, exports are available at the time when babel transforms stories. I've updated the plugin to use named exports. Thank you for the suggestion!

@dmytrokirpa dmytrokirpa changed the title fix(public-docsite-v9): fix positioning components stories fix(babel-full-source-plugin): use named exports for stories detection Aug 12, 2024
@dmytrokirpa dmytrokirpa changed the title fix(babel-full-source-plugin): use named exports for stories detection fix(babel-preset-storybook-full-source): use named exports for story detection Aug 12, 2024

@Hotell Hotell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 actionable and 1 question. beside that great job !

Comment thread packages/react-components/babel-preset-storybook-full-source/src/fullsource.ts Outdated
@dmytrokirpa
dmytrokirpa force-pushed the fix_docsite_positioning_stories branch from 7b64bf2 to 8ed1fac Compare August 13, 2024 11:19
@dmytrokirpa
dmytrokirpa requested a review from Hotell August 13, 2024 11:19
@dmytrokirpa
dmytrokirpa merged commit 48c6239 into microsoft:master Aug 13, 2024
@dmytrokirpa
dmytrokirpa deleted the fix_docsite_positioning_stories branch August 13, 2024 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants