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

Allow environment overrides specifically for Storybook #87

Merged
merged 3 commits into from
Jul 3, 2022

Conversation

DingoEatingFuzz
Copy link
Contributor

Stories are rendered as full Ember Apps including the default environment, which may have settings that aren't appropriate for only rendering a single story in an iframe.

This makes it so settings in environment.js can be overridden by settings in .storybook/environment.js.

It also resets rootURL to / (after doing the existing rootURL resource transformations) since Ember doesn't at all like running the router in an iframe when rootURL is anything other than /.

index.js Outdated
@@ -93,6 +94,22 @@ module.exports = {

this.ui.writeDebugLine('Generating preview-head.html');

const environment = parsedConfig.meta.find(meta => meta.name.endsWith('config/environment'));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we write a test case for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep! Just wanted to make sure you were cool with the feature first 😅

@dgeb
Copy link

dgeb commented Apr 11, 2022

I'm also running into issues running storybook with a rootURL other than /. @DingoEatingFuzz thanks for putting this PR together - it seems like a good approach. Are you up for resolving the conflicts and adding some tests?

Stories are rendered as full Ember Apps including the default
Environment which may have settings that aren't appropriate for only
rendering a single story in an iframe.

Now those can be overridden with a .storybook/environment.js file
@DingoEatingFuzz
Copy link
Contributor Author

Sorry about the delay. I rebased and added some tests.

There weren't existing tests for index.js (which I get, since it would require mocking all the build stuff) so I refactored the implementation a bit to push more of the logic into utils. Hopefully that is sufficient coverage.

@gossi
Copy link
Contributor

gossi commented Jul 3, 2022

Looking good to me. Going to hit the merge button 👍

Thanks @DingoEatingFuzz for your PR and patience 😊

@gossi gossi merged commit 44fe13f into storybookjs:master Jul 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants