Add style-loader and prevent css-loader from rewriting external URLs#1394
Closed
jamiebenstead wants to merge 19 commits into
Closed
Add style-loader and prevent css-loader from rewriting external URLs#1394jamiebenstead wants to merge 19 commits into
jamiebenstead wants to merge 19 commits into
Conversation
jamiebenstead
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the webpack configuration to ensure third-party .css imports are injected at runtime (and their referenced assets, like fonts, resolve correctly), as an attempted fix for issue #1243.
Changes:
- Add
style-loaderto the.cssloader pipeline so CSS imports are injected into the page at runtime. - Minor formatting/consistency fixes in
webpack.config.js(trailing commas/parentheses).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
jamiebenstead
changed the title
This is the result of a few hours iteration to remove noise from the console. RaspberryPiFoundation/digital-editor-issues#1140 Fixes applied: - Silent console.warn("PyodideWorker is not initialized") as I'm not sure if brings much value - defaultIndex not used an invalid prop - Missing key in DesignSystemButton - silent webpackOk - destruct in [ScratchIntegrationHOC.jsx](https://github.com/RaspberryPiFoundation/editor-ui/compare/issues/1440?expand=1#diff-a19991532270c68dd2e8d9724452e4e3883f710e43c90dbc1e0af33bea31df13) to avoid saying not valid props - icon warning not needed favicon since its in iframe - Two dedupers to avoid error/warning flooding, approached this way to be still throwing the warning but be easily removable: - Deduping errors coming from design system, needs to be addressed in source - Deduping errors coming from scratch they should be fixed in origin, or when updating https://github.com/user-attachments/assets/fb919074-56f6-478e-a3ff-456d8fa6f640
Bumps [undici](https://github.com/nodejs/undici) from 5.28.4 to 5.29.0. - [Release notes](https://github.com/nodejs/undici/releases) - [Commits](nodejs/undici@v5.28.4...v5.29.0) --- updated-dependencies: - dependency-name: undici dependency-version: 5.29.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Closes: RaspberryPiFoundation/digital-editor-issues#1230 Updated Scratch to version 13 and update to react-redux 8 is needed since it's a required peer dependency. Also for react-redux 8 we need redux as a peer dependency too, so we need to provide it ourselves. Updating the version didn't change the HTML preview behavior directly, but it changed the timing enough to expose an existing race in the HTML runner internal-link flow. #### Test change explanation: When an internal link was clicked, the app updated previewFile and triggered a rerun in the same tick. In CI runs, because it can be slower, the rerun could happen before React had actually applied the new previewFile, so the iframe sometimes re-rendered new.html again instead of index.html. #### What changed: - In externalLinkHelper internal-link handling now only updates `previewFile`. In HtmlRunner, the rerun is deferred until after previewFile has actually changed, via `reloadAfterPreviewChange`. - Also same-file reloads are now handled explicitly, so if the requested preview file is already the current one, we rerun immediately instead of leaving a pending reload behind. - In spec-html.cy, `getIframeBody()` doesn't use` .then(cy.wrap)`, so Cypress keeps re-querying the live iframe body instead of holding a stale body element across iframe navigation. - The assertion in allows internal links was simplified to getIframeBody().should("contain.text", "hello world"), which matches the user-visible behavior and avoids depending on a specific node during reload. Now the assertion is less coupled to the exact DOM shape and retries against the current iframe document, which is what we want for navigation/reload behavior. There was also an issue exposed in WebComponentLoader after the dependency update. `loadRemix` was being kept as derived state through an effect, and after the upgrade that became unstable. Now `loadRemix` is derived directly from user and `remixLoadFailed`, which removes the extra state sync and avoids the loader getting stuck. In WebComponentLoader.test, the mocked `changeLanguage` now resolves instead of returning a never-ending promise, which matches the real behavior better and avoids the test hanging.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts the Webpack build pipeline so third-party and app CSS can be injected at runtime (not just transformed) and aims to avoid css-loader rewriting external URLs (notably font URLs), as part of addressing raspberrypi/digital-editor-issues#1243.
Changes:
- Add
style-loaderto the.cssloader chain so imported CSS is injected into the page at runtime. - Minor formatting fixes (missing semicolons / trailing comma in
devServer.staticconfig).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
37
to
+39
| test: /\.css$/, | ||
| use: ["css-loader"], | ||
| use: [ | ||
| "style-loader", |
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.
Add style-loader to ensure CSS (and font URLs) are correctly injected and resolved at runtime and prevent css-loader from rewriting external URLs
This is an attempted fix for https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/1243