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/12745 babel config loading #15628

Closed
wants to merge 7 commits into from
Closed

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Jul 19, 2021

Issue: #14425 #2758 #12745 #15502

What I did

  • removed the docs telling users babelrc in project root would be loaded (it isn't)
  • added support for loading babel.config.ts files in storybook config directory
  • added support for the babel config file exporting a function
  • layer the config resolution from main.js and the babel config files

How to test

  1. create a storybook
  2. expect the babelconfig to be the default
  3. add a .storybook/babel.config.ts which exports a function (returning valid babel config)
  4. run storybook and expect the babel config to be used which was specified in the above config file; the default config is not used
  5. add a babel export to main.js with valid babel config ( a function that takes config and returns changed config)
  6. run storybook and expect a the babel config to be merged with main.js overriding babel.config.ts; the default config is not used

@ndelangen ndelangen self-assigned this Jul 19, 2021
@nx-cloud
Copy link

nx-cloud bot commented Jul 19, 2021

Nx Cloud Report

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

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

…hat it got from the file, if no file return null

perform or operation in the common-preset, determines the default babel config object
@ndelangen ndelangen marked this pull request as ready for review July 21, 2021 12:45
@ndelangen ndelangen requested review from yannbf, domyen and gaetanmaisse and removed request for domyen July 21, 2021 12:46
@ndelangen ndelangen added this to the 6.4 improvements milestone Jul 21, 2021
If your project has a `.babelrc` file, we'll use that instead of the default config file.

You can also place a `.storybook/.babelrc` file to use a special configuration for Storybook only.
If your project has a `.storybook/.babelrc` file, we'll use that instead of the default config file.

Choose a reason for hiding this comment

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

According to the code https://github.com/storybookjs/storybook/blob/next/lib/core-common/src/utils/load-custom-babel-config.ts#L79 there are more filenames considered for loading in the .storybook folder

  • .babelrc
  • .babelrc.json
  • .babelrc.js
  • babel.config.json
  • babel.config.js

Copy link
Member

Choose a reason for hiding this comment

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

Important to note as well that the recommended setting for Babel 7 is to use babel.config.json, lots of people are renaming their configuration file so it would be nice indeed to see that extension in the docs

Ref: https://babeljs.io/docs/en/config-files#6x-vs-7x-babelrc-loading

@@ -48,10 +49,16 @@ function loadFromPath(babelConfigPath: string): TransformOptions {
throw error.js;
}

config = { ...config, babelrc: false };
if (config instanceof Function) {
config = config();

Choose a reason for hiding this comment

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

Calling the config function without an argument could be problematic. Babel calls the function with an api object https://babeljs.io/docs/en/config-files#config-function-api
In my opinion it is ok to stub that api and make sure that calls to the methods don't fail. The env method should be easy to reimplement.

@shilman shilman modified the milestones: 6.4 improvements, 6.4 PRs Jul 22, 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
@shilman
Copy link
Member

shilman commented Jan 14, 2022

@ndelangen I believe we fixed this with the babelModeV7 fix https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#babel-mode-v7

Closing this for now. Please let me know if there's more to be done here!

@shilman shilman closed this Jan 14, 2022
@stof stof deleted the fix/12745-babel-config-loading branch May 25, 2022 09:43
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.

4 participants