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

StorybookConfig: Add babel field #15220

Closed

Conversation

tyankatsu0105
Copy link
Contributor

Issue:

What I did

How to test

  • Is this testable with Jest or Chromatic screenshots? yes
  • Does this need a new example in the kitchen sink apps? no
  • Does this need an update to the documentation? no

If your answer is yes to any of these, please make sure to include it in your PR.

@nx-cloud
Copy link

nx-cloud bot commented Jun 13, 2021

☁️ Nx Cloud Report

CI ran the following commands for commit 0e3bf30. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@tyankatsu0105
Copy link
Contributor Author

yarn build

Built: @storybook/core-common@6.3.0-rc.4
lerna ERR! yarn run prepare exited 1 in '@storybook/core-server'
lerna ERR! yarn run prepare stdout:
Successfully compiled 28 files with Babel (1171ms).

Successfully compiled 28 files with Babel (1175ms).

Successfully compiled 28 files with Babel (1320ms).

src/presets/babel-cache-preset.ts(11,14): error TS2742: The inferred type of 'babel' cannot be named without a reference to '@babel/parser/node_modules/@babel/types'. This is likely not portable. A type annotation is necessary.
src/presets/babel-cache-preset.ts(11,14): error TS4023: Exported variable 'babel' has or is using name 'InputSourceMap' from external module "/Users/tyankatsu/project/storybook/lib/core-server/node_modules/@types/babel__core/index" but cannot be named.
src/presets/babel-cache-preset.ts(12,14): error TS2742: The inferred type of 'managerBabel' cannot be named without a reference to '@babel/parser/node_modules/@babel/types'. This is likely not portable. A type annotation is necessary.
src/presets/babel-cache-preset.ts(12,14): error TS4023: Exported variable 'managerBabel' has or is using name 'InputSourceMap' from external module "/Users/tyankatsu/project/storybook/lib/core-server/node_modules/@types/babel__core/index" but cannot be named.

@tyankatsu0105
Copy link
Contributor Author

I don't know the solution that fixable this error...

@alvis
Copy link

alvis commented Aug 13, 2021

You see this error because TransformOptions is from @types/babel__core but it's installed as a devDependency.
To share types from other packages, you have to include the @type package as a typical dependency otherwise it won't be installed by the end user project.

It's a bit annoying to have @types/babel__core added as a dependency for just a simple type.
Possibly you can just copy & paste the interface from @types/babel__core to here.

@alvis
Copy link

alvis commented Aug 15, 2021

@tyankatsu0105 I saw that you tried to add the type package to dependencies but the test still failed. It's likely that because the lock file hasn't been updated so the CI still picked up the old setting.

@shilman shilman changed the title feat: add babel field to StorybookConfig StorybookConfig: Add babel field Aug 17, 2021
@tyankatsu0105
Copy link
Contributor Author

@shilman Do you know the solution of CI's error?
I don't know what should I do...

@alvis
Copy link

alvis commented Aug 18, 2021

@tyankatsu0105 Seems like it's an issue fixed in #15863
Maybe you can force push again to trigger another test?

@shilman shilman self-assigned this Aug 30, 2021
@alvis
Copy link

alvis commented Sep 10, 2021

@shilman It appears that it's a lock file problem that makes CI tests fail. But I can't find anywhere that uses @babel/core@7.14.8, any clue?

@shilman shilman assigned darleendenno and unassigned darleendenno Sep 13, 2021
@stale
Copy link

stale bot commented Jan 9, 2022

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

@stale stale bot added the inactive label Jan 9, 2022
@stale stale bot removed the inactive label Jan 24, 2022
@ndelangen
Copy link
Member

Hello @tyankatsu0105 I'm unfortunately the bringer of bad news :(

I really want to thank you for this PR. I know it can't have been easy, figuring our webpack & babel config and getting that changed in storybook. Trust me I know. So I know you've put a lot of effort, passion and time into this. So it feels super bad to be closing this PR :(

But here's the thing... In 6.x we can't really change any of this, because whatever we do to add babel stuff or not add it, change versions... set config or not set config... it always breaks someone's setup somewhere..

So in 6.4 we introduced a feature flag called babelModeV7 and this effectively turns off a load hacks from storybook's side to make it compatible with most things out there.

In 7.0 we can finally change things and we've opted to make babelModeV7 the new default and make it even clearer: we let the babel loader do whatever is in it's nature, it will let babel resolve config the way babel resolves config. We do not try to add config to babel, unless it's something we specifically need to do.

The upside of these changes is that we get far less version conflicts (or rather USERS get less of them) and we also make storybook lighter to install (which is a common thing we hear "storybook being bloated").

We're actively working on making storybook easier to maintain for ourselves, and slimmer to install for users, and simpler to configure for all. Less babel config is part of that.

I hope you understand why I'm closing this PR. Again: thank you for the time and energy you spend on this, I'm sorry we couldn't merge it. If you feel like I'm in the wrong here, please let me know, happy to discuss this further any time (for example on discord if you prefer)

@ndelangen ndelangen closed this Jun 30, 2022
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.

None yet

5 participants