fix: explicitly set publicPath for static assets#7
Merged
sebjacobs merged 3 commits intoexperience-csfrom Apr 16, 2025
Merged
Conversation
floehopper
approved these changes
Apr 16, 2025
There was a problem hiding this comment.
Great detective work - I found the scratch-webpack-configuration code/config very confusing when I looked at it previously!
This looks good to me - assuming the build errors can be resolved 👍
I think we should probably have a wider discussion about how much effort we're going to put in to making our changes upstream-able. e.g. I notice that the recent change to set the default stage size is hard-coded rather than configurable.
c41a529 to
920fc6b
Compare
920fc6b to
ad5e946
Compare
STATIC_PATH was introduced back in 2019 [1] as a way to allow consumers of scratch-gui (like ourselves) to customize the publicPath (where assets are served from when the JS is run in the browser). However it looks like this feature was removed/commented out [2] when they migrated to using the shared webpack config (from the npm package `scratch-webpack-configuration`). We would like to re-introduce this feature in order to fix an issue we have noticed with image assets failing to load as the browser is attempting to load thse assets from URLs relative to the webpack entry point (rather than from the root or `/scratch-gui`). Therefore I am removing this commented out code to make the existing config less confusing. [1] scratchfoundation@e2a2dcf0f [2] scratchfoundation@f1438f73e Co-Authored-By: Chris Lowis <chris.lowis@gofreerange.com>
The existing webpack config customizes the filenames of any assets we import which have not been inlined using the `assetModuleFilename`. This config option is defined at the global level, however it only applies to assets that are processed by the Asset module definition. Rather than configuring this globally, I have opted for defining it within the Asset module definition (nested under the `generator` option) as that makes it clearer that this option is only relevant for those assets thus making it easier for future developers to understand and make changes in the future, especially for those who are less familiar with the intricacies of webpack config. [1] https://webpack.js.org/guides/asset-modules/ Co-Authored-By: Chris Lowis <chris.lowis@gofreerange.com>
ad5e946 to
d1b3a4f
Compare
https://github.com/RaspberryPiFoundation/experience-cs/issues/23 We recently embedded the scratch-gui within our Experience CS app. We have since noticed a number of issues with assets failing to load within the browser, for example the browser is attempting to load assets from URLs relative to the webpack entry point (rather than from the root or from `/scratch-gui/`). After some investigation it turns out that is due to the fact that the webpack option `publicPath` had not been explicity set [1] (which apparently default to `auto`). From digging into the webpack docs [2], it looks like when `publicPath` is set to `auto` it will attempt to load assets relative to the entrypoint which is not what we want. For our purposes we want assets to be served from `/scratch-gui`, and I can't see the need to make this configurable at this stage, therefore I have hardcoded this value for now as that also makes building the package less prone to error. However if we were to push changes upstream, then I think it would make sense to use an ASSET_PATH environment variable. Rather than configuring this globally, I have opted for defining it within the Asset module definition (nested under the `generator` option) as that makes it clearer that this option is only really relevant for those assets thus making it easier for future developers to understand and make changes in the future, especially for those who are less familiar with the intricacies of webpack config. I also dug into the latest version of scratch-gui and scratch-webpack-configuration and can confirm that this would still be an issue even with the latest releases, but at least they are now explicit about setting `publicPath` to `auto`, rather than the config option missing entirely and falling back to the default value of `auto` [3]. [1] https://github.com/scratchfoundation/scratch-webpack-configuration/blob/v1.5.1/src/index.cjs#L71 https://github.com/RaspberryPiFoundation/scratch-gui/blob/b723be4d1/webpack.config.js#L20 [2] https://webpack.js.org/guides/public-path/ ``` **Automatic publicPath** There are chances that you don't know what the publicPath will be in advance, and webpack can handle it automatically for you by determining the public path from variables like import.meta.url, document.currentScript, script.src or self.location. ``` [3] https://github.com/scratchfoundation/scratch-gui/blob/853caca41/webpack.config.js#L29 Co-Authored-By: Chris Lowis <chris.lowis@gofreerange.com>
d1b3a4f to
acaed96
Compare
sebjacobs
pushed a commit
that referenced
this pull request
Apr 24, 2025
sebjacobs
pushed a commit
that referenced
this pull request
Apr 30, 2025
sebjacobs
pushed a commit
that referenced
this pull request
Apr 30, 2025
sebjacobs
pushed a commit
that referenced
this pull request
Apr 30, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://github.com/RaspberryPiFoundation/experience-cs/issues/23
We recently embedded the scratch-gui within our Experience CS app.
We have since noticed a number of issues with assets failing to load within the browser, for example the browser is attempting to load assets from URLs relative to the webpack entry point (rather than from the root or from
/scratch-gui/).After some investigation it turns out that is due to the fact that the webpack option
publicPathhad not been explicity set [1] (which apparently default toauto).From digging into the webpack docs [2], it looks like when
publicPathis set toautoit will attempt to load assets relative to the entrypoint which is not what we want.For our purposes we want assets to be served from
/scratch-gui, and I can't see the need to make this configurable at this stage, therefore I have hardcoded this value for now as that also makes building the package less prone to error.However if we were to push changes upstream, then I think it would make sense to use an ASSET_PATH environment variable.
Rather than configuring this globally, I have opted for defining it within the Asset module definition (nested under the
generatoroption) as that makes it clearer that this option is only really relevant for those assets thus making it easier for future developers to understand and make changes in the future, especially for those who are less familiar with the intricacies of webpack config.I also dug into the latest version of
scratch-guiandscratch-webpack-configurationand can confirm that this would still be an issue even with the latest releases, but at least they are now explicit about settingpublicPathtoauto, rather than the config option missing entirely and falling back to the default value ofauto[3].[1]
https://github.com/scratchfoundation/scratch-webpack-configuration/blob/v1.5.1/src/index.cjs#L71
https://github.com/RaspberryPiFoundation/scratch-gui/blob/b723be4d1/webpack.config.js#L20
[2]
https://webpack.js.org/guides/public-path/
[3]
https://github.com/scratchfoundation/scratch-gui/blob/853caca41/webpack.config.js#L29