Skip to content

Conversation

@ktbee
Copy link
Contributor

@ktbee ktbee commented Dec 11, 2018

Resolves

When I tried to use the default SVG asset in scratch-storage's builtins folder, I found that the data property just returned a string that was the file's path and name. This prevented me from getting the file's contents.

Proposed Changes

Disable the Webpack file-loader loader for these require statements. Webpack will still use the specified arraybuffer-loader loader for these files, but not using file-loader will give arraybuffer-loader the contents of the files rather than a string with the path and name of the file.

@thisandagain
Copy link
Contributor

/cc @cwillisf @rschamp

Copy link
Contributor

@cwillisf cwillisf left a comment

Choose a reason for hiding this comment

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

I'm OK with these changes but I'm confused about the need since there's no mention of file-loader in webpack.config.js. In the interest of my own learning: is file-loader somehow included in the default Webpack config, and if so is that documented somewhere? (I can't seem to find it...)

@ktbee
Copy link
Contributor Author

ktbee commented Dec 12, 2018

@cwillisf that's a good question. I believe the rule for loading SVGs with file-loader in the scratch-gui is what's affecting what the builtin assets return. This is the relevant line in the gui's Webpack configuration: https://github.com/LLK/scratch-gui/blob/055b2fc541c3a40a1d432a791144b883bb3ec4be/webpack.config.js#L115-L121

@cwillisf
Copy link
Contributor

@ktbee aha! That makes sense. Thanks!

@ktbee
Copy link
Contributor Author

ktbee commented Dec 13, 2018

Also here is the relevant Webpack documentation, for anyone that's not familiar with the ! convention: https://github.com/webpack/docs/wiki/loaders#loader-order

Copy link
Contributor

@kchadha kchadha left a comment

Choose a reason for hiding this comment

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

@ktbee thanks for linking to the documentation. LGTM!

@kchadha kchadha assigned ktbee and unassigned kchadha Dec 13, 2018
@ktbee ktbee merged commit bce862d into scratchfoundation:develop Dec 13, 2018
@rschamp
Copy link
Contributor

rschamp commented Dec 13, 2018

🎉 This PR is included in version 1.2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

5 participants