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

Add support for multiple webpack chunks in iframe #677

Closed
wants to merge 2 commits into from

Conversation

apexskier
Copy link
Contributor

This allows an arbitrary number of files to be loaded into the iframe, not just the single preview.bundle.js file.

Webpack plugins can output chunks in a separate file. In dev, since file names are predictable, I can add the js file into a head.html file. When building storybook statically, however, js files have a hash added to them, so this doesn't work.

This change makes the static build parse all webpack chunks and load them in the iframe (excluding the manager script), instead of only including known files.

This allows an arbitrary number of files to be loaded into the iframe,
not just the single `preview.bundle.js` file.

Webpack plugins can output chunks in a separate file. In dev, since
file names are predictable, I can add the js file into a head.html file.
When building storybook statically, however, js files have a hash added
to them, so this doesn't work.

This change makes the static build parse all webpack chunks and load
them in the iframe (excluding the manager script), instead of only
including known files.
@ndelangen
Copy link
Member

Interesting! I'll look at it!

@ndelangen ndelangen self-requested a review March 28, 2017 16:11
@@ -8,34 +8,43 @@ import url from 'url';
// 'preview.0d2d3d845f78399fd6d5e859daa152a9.css',
// 'static/preview.9adbb5ef965106be1cc3.bundle.js.map',
// 'preview.0d2d3d845f78399fd6d5e859daa152a9.css.map' ]
const previewUrlsFromAssets = (assets) => {
const urlsFromAssets = (assets) => {
if (!assets) {
Copy link
Member

Choose a reason for hiding this comment

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

Do you think you could refactor this into a single return statement? I think this function has become too complex.

@ndelangen
Copy link
Member

Hey @apexskier Do you think you can take another look at this?

@ndelangen
Copy link
Member

I have no access to your fork, So I'll create a new local branch and create a new PR from there.

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.

3 participants