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

Handle loading .storybook/babel.config.js (#6633) #6634

Merged
merged 3 commits into from
May 20, 2019
Merged

Handle loading .storybook/babel.config.js (#6633) #6634

merged 3 commits into from
May 20, 2019

Conversation

LukeAskew
Copy link
Contributor

@LukeAskew LukeAskew commented Apr 25, 2019

@vercel
Copy link

vercel bot commented Apr 25, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://monorepo.storybook.now.sh

@shilman shilman added this to the 5.0.x milestone Apr 25, 2019
@shilman shilman added bug patch:yes Bugfix & documentation PR that need to be picked to main branch labels Apr 25, 2019
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Great fix! Did you test it?

@ndelangen are there any good places to drop in tests for this?

@ndelangen
Copy link
Member

@LukeAskew Would you be able to see what we can do to fix the failing cli test?
https://circleci.com/gh/storybooks/storybook/121204

@LukeAskew
Copy link
Contributor Author

LukeAskew commented Apr 26, 2019

Sure, but I could use a little guidance.

  1. These same cli tests fail locally when I run at the HEAD of next. I think this is because the babel configs in these fixtures don't include preset-react. I am unsure of why these pass on Circle.
  2. What is the intended behavior of test cases like this? I would expect this config to be used instead of the default and I would expect this to fail because it lacks the necessary presets.
  3. How can I link my changes in core to the cli test? It appears that the fixtures pull directly from npm. It is unclear how I can test my changes across the repo.

@igor-dv
Copy link
Member

igor-dv commented May 1, 2019

AFAIR, The similar fix was already considered some time ago. One problem is that it might be a breaking change. SB could start requiring babel configs from root, that are existing in the project and where not required before.

Also some discussion was here - #2582

@LukeAskew
Copy link
Contributor Author

LukeAskew commented May 9, 2019

Updated the code to just handle .storybook/babel.config.js in addition to .babelrc. This allows folks to piggyback on the root config via:

module.exports = require('../babel.config');

@LukeAskew LukeAskew changed the title Ensure custom babel configs are loaded (#6633) Handle loading .storybook/babel.config.js (#6633) May 9, 2019
@ndelangen ndelangen self-assigned this May 18, 2019
@ndelangen ndelangen merged commit a08f44e into storybookjs:next May 20, 2019
@shilman shilman removed the patch:yes Bugfix & documentation PR that need to be picked to main branch label May 20, 2019
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