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

3.3.0-alpha.4 preview-head does not work with a custom configDir #2501

Closed
skovy opened this issue Dec 18, 2017 · 9 comments
Closed

3.3.0-alpha.4 preview-head does not work with a custom configDir #2501

skovy opened this issue Dec 18, 2017 · 9 comments

Comments

@skovy
Copy link
Contributor

skovy commented Dec 18, 2017

Issue details

On 3.3.0-alpha.4, we use a custom config directory (frontend/.storybook). This works on 3.2, but is broken on 3.3.0-alpha.4. This appears to be fixed on master but not release/3.3. It appears the ENV var is always undefined:

https://github.com/storybooks/storybook/blob/3c1715dacaf95a789e4e024cdb1e8993bf3ada9b/app/react/src/server/config/utils.js#L37

Steps to reproduce

Add a custom config directory (start-storybook -c frontend/.storybook -p 6006), with a preview-head.html file in the config directory (frontend/.storybook/preview-head.html).

Temporary Solution

I was able to work around this issue for the time being by adding this config var:

SBCONFIG_CONFIG_DIR=frontend/.storybook start-storybook -c frontend/.storybook -p 6006

Please specify which version of Storybook and optionally any affected addons that you're running

  • @storybook/react@3.3.0-alpha.4

Affected platforms

  • All.
@shilman
Copy link
Member

shilman commented Dec 27, 2017

I'm able to reproduce this exactly as described here by @skovy. Looking into a fix.

cc @danielduan

@samouss
Copy link
Contributor

samouss commented Dec 27, 2017

The issue seems to come from the fact that in the webpack.config the configDir is always read from the ENV instead of coming from the program (see config/webpack.config.js#L37-L53). If you follow the call to getConfigDir you will see that the function always read the configDir from the ENV or the default one (see config/utils.js#L37).

Since the webpack.config is a function we can probably pass the configuration directory as argument and get rid of the getConfigDir function. The webpack.config function is called from the middleware module (see server/middleware.js), we have access to the configDir variable so we can pass it as argument. Not sure of the implication, but I tried and it works.

Happy to send a PR if you think that it's a viable solution.

@shilman
Copy link
Member

shilman commented Dec 28, 2017

@samouss I have also tried this fix out and have something basic working, but don't understand the implications of the fix. If you could put together a PR and we can get other people on the team to comment, that would be really awesome!!

@danielduan
Copy link
Member

I'm quite confused as to why this was working before and not anymore. I think there might be a PR somewhere to enable configuration via environment variables.

@samouss
Copy link
Contributor

samouss commented Dec 28, 2017

It seems that a recent change introduce the HtmlWebpackPlugin for generate both manager & iframe html files. Previously they was generate in the build.js file where the configDir was available in the current scope (see f422481#diff-ba110c4fe305160aa94df6d17e4235dcL102).

But now they are generated form the webpack.config.js file where configDir is not available. So the function getConfigDir has been created and that's where the problem come from (see f422481#diff-917756e90ff65c053bce52389df3c81bR49).

#1775

@shilman
Copy link
Member

shilman commented Jan 6, 2018

@samouss Any progress on a PR for this?

@samouss
Copy link
Contributor

samouss commented Jan 6, 2018

Nope, I didn't have the time to work on it yet. But I hope I will have it in the coming days!

@shilman
Copy link
Member

shilman commented Jan 7, 2018

Fixed in #2666

@Hypnosphi
Copy link
Member

Fix released as 3.3.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants