Fix [email protected] for browser builds #421
Merged
Conversation
This file contains 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
This checks for things like accidentally using the filename as the file content, etc.
AlexisGoodfellow
approved these changes
Oct 13, 2023
|
The "good" news is: if this doesn't fix the problem, |
|
🎉 This PR is included in version 2.3.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
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.
Resolves
Proposed Changes
Update the
browserfield inpackage.jsonto point todist/web/scratch-storage.jsinstead ofsrc/index.jsReason for Changes
Browsers don't understand
webpackloaders, so strictly speaking, this field has been incorrect since we first implemented default assets. We didn't notice because, within Scratch repositories, we always build withwebpack. Removing the inline loader syntax in #412 made it so that dependents need to specify loader rules, and thus brokescratch-guiand others. Telling dependents to use the build output makes it so the loader is applied as part of thescratch-storagebuild process and doesn't need to be configured in every dependent. Also, FLUFFY FISHSTICKS this took SO much longer than it should have. It's exceptionally difficult to fix this at thescratch-guilevel, which is what I was beating my head against for a few days. I'm very happysad that it's effectively a one-liner inscratch-storage.Test Coverage
I enhanced the
load-default-assetstest to verify the exact size of each default asset. That way, we can be sure that the file contents are accurate instead of, for example, a filename or a data URI string. Basically, it now tests that the loader is doing its job and configured correctly. To do this, it needs to use the build output instead of the raw source.