Prepare webpack app for Vite migration#1508
Draft
zetter-rpf wants to merge 13 commits into
Draft
Conversation
Move browser-facing environment lookups behind runtime helpers so the upcoming Vite migration only has to adapt one compatibility layer instead of many scattered process.env call sites. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Capture the webpack build outputs that deployments and Cypress already depend on so the later Vite migration has an executable contract to preserve. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Move copy patterns and cross-origin header rules into reusable config so Vite can preserve the same deployable artifacts and Pyodide/Scratch isolation behavior during the bundler switch. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Mark SCSS and Markdown imports that are consumed as strings so Vite can preserve those semantics without relying on webpack loader defaults. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Move Scratch CSP origin and template parameter generation into a tested helper so a future Vite HTML transform can reuse the same deployment security rules. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Keep the stable PyodideWorker.js artifact contract in one tested helper so Vite output changes can be handled without touching the runner lifecycle. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Keep the initial Vite migration focused on the bundler by documenting why Jest should remain in place until build parity is proven. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Keep the Vite-prep runtime path helper calls aligned with the repository ESLint/Prettier rules so validation stays focused on behavior. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Match the production webpack copy contract by checking Scratch chunks under build/chunks, while leaving the dev-server scratch-gui/chunks path to the server static config. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Prevent blank HTML_RENDERER_URL or ASSETS_URL values from producing invalid postMessage origins during the Vite prep runtime-config abstraction. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Blob workers cannot importScripts root-relative paths, so use the browser origin when PUBLIC_URL is empty while preserving the stable PyodideWorker.js artifact contract. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
Preserve HTML preview behavior when allowed iframe origins are not configured by accepting messages from the same origin as the renderer iframe. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
|
Cursor Agent can help with this pull request. Just |
zetter-rpf
temporarily deployed
to
previews/1508/merge
GitHub Actions
Inactive
Contributor
There was a problem hiding this comment.
Pull request overview
Prepares the existing webpack-based build/runtime for an eventual Vite migration by centralizing environment + URL handling, making “string asset” imports explicit via query semantics, and extracting build/development-server contracts into reusable helpers (with smoke-test coverage for emitted artifacts).
Changes:
- Added
runtimeConfig/ URL helper utilities and updated callers to avoid directprocess.env.*usage in app code. - Extracted webpack copy patterns, Scratch template CSP/origin logic, and dev-server cross-origin rules into shared modules.
- Added a build artifact smoke-check script and updated Jest mapping for
?raw/?inlineimports.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| webpack.config.js | Uses shared helpers for Scratch dirs/copy patterns, Scratch template parameters, and dev-server CORS/CORP handling. |
| src/web-component.js | Reads Sentry config via getRuntimeEnv. |
| src/utils/userManager.js | Reads OIDC client id via getRuntimeEnv. |
| src/utils/setupTests.js | Updates markdown mock to match ?raw import form. |
| src/utils/scratchTemplateConfig.test.js | Adds unit tests for Scratch template CSP/origin helpers. |
| src/utils/scratchTemplateConfig.cjs | New shared Scratch template parameter/origin utilities for webpack + runtime reuse. |
| src/utils/scratchIframe.js | Uses getAssetsUrl() instead of process.env.ASSETS_URL. |
| src/utils/runtimeConfig.test.js | Adds unit tests for env/url helper behavior and fallbacks. |
| src/utils/runtimeConfig.js | New centralized runtime env + URL joining helpers. |
| src/utils/pyodideWorkerUrl.test.js | Adds tests covering worker script URL + blob bootstrap creation. |
| src/utils/pyodideWorkerUrl.js | New helper for Pyodide worker script URL + blob bootstrap generation. |
| src/utils/iframeUtils.test.js | Adds tests for iframe-origin allowlist behavior (incl. same-origin). |
| src/utils/iframeUtils.js | Uses runtime config + allows same-origin; parses allowlist env var. |
| src/utils/i18n.js | Uses publicPath() for translation loadPath. |
| src/utils/externalLinkHelper.js | Uses getPublicUrl() for internal link host base. |
| src/utils/dedupeScratchWarnings.js | Uses isDevelopment() helper. |
| src/utils/dedupeDesignSystemWarnings.js | Uses isDevelopment() helper. |
| src/scratch.jsx | Removes process import, uses isProduction(), and makes Scratch SCSS import explicit with ?inline. |
| src/PyodideWorker.js | Uses assetPath() for local shim and package URLs. |
| src/containers/WebComponentLoader.jsx | Uses ?inline stylesheet imports and getRuntimeEnv defaults for endpoints. |
| src/components/ScratchEditor/ScratchEditor.jsx | Reuses exported Scratch library asset URL template and uses assetPath() for GUI basePath. |
| src/components/Menus/Sidebar/InstructionsPanel/InstructionsPanel.jsx | Updates markdown import to ?raw form. |
| src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx | Uses assetPath() for shims/libs and publicPath() for copydecks base. |
| src/components/Editor/Runners/PythonRunner/PyodideRunner/PyodideRunner.jsx | Replaces inline worker blob creation with shared helper; uses publicPath() for copydecks base. |
| src/components/Editor/Runners/HtmlRunner/HtmlRunner.jsx | Uses runtime-config renderer origin + htmlRendererPath() for iframe src. |
| src/components/Editor/Runners/HtmlRunner/HtmlRenderer.test.js | Updates tests to use runtime-config renderer URL (and sets env in test). |
| src/components/Editor/Runners/HtmlRunner/HtmlRenderer.jsx | Uses ?inline styles and caches renderer URL for postMessage origin in generated handlers. |
| src/components/Editor/Project/ScratchContainer.jsx | Uses assetPath() to build Scratch iframe URL with query params. |
| src/components/AstroPiModel/FlightCase.jsx | Uses assetPath() for GLB model URL. |
| scripts/check-build-artifacts.js | New script to assert required build outputs exist under build/. |
| package.json | Adds check-build-artifacts script and Jest mappers for ?raw / ?inline. |
| docs/ViteMigration.md | Documents keeping Jest during initial Vite bundler migration scope. |
| config/devServerSecurity.js | New shared dev-server cross-origin header + CORP middleware helper. |
| config/buildArtifacts.js | New shared webpack copy patterns and Scratch artifact directory helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
+8
| const trimLeadingSlash = (value) => value.replace(/^\/+/, ""); | ||
|
|
||
| export const getRuntimeEnv = (name, fallback = "") => { | ||
| const env = typeof process === "undefined" ? {} : process.env || {}; | ||
| const value = env[name]; | ||
|
|
||
| return value === undefined ? fallback : value; | ||
| }; |
Comment on lines
+7
to
+10
| const allowedHosts = getRuntimeEnv("REACT_APP_ALLOWED_IFRAME_ORIGINS") | ||
| .split(",") | ||
| .filter(Boolean); | ||
| return isTest() || isSameOrigin(origin) || allowedHosts.includes(origin); |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b21bec2. Configure here.
| const value = env[name]; | ||
|
|
||
| return value === undefined ? fallback : value; | ||
| }; |
There was a problem hiding this comment.
Dynamic env lookup breaks webpack
High Severity
getRuntimeEnv reads process.env with a dynamic key, but the webpack dotenv-webpack setup only inlines static process.env.VAR references at build time. In browser and worker bundles, configured values like ASSETS_URL, API endpoints, and Sentry settings can resolve to empty fallbacks instead of the baked-in production values the previous code received.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit b21bec2. Configure here.
Provide a bundled runtime env object so the prep branch's central config helper works with webpack's static env replacement and blob workers can resolve absolute asset URLs in Cypress. Co-authored-by: Chris Zetter <zetter-rpf@users.noreply.github.com>
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.
Summary
?inlineand?rawsemantics.globalThis.location.originas the runtime origin fallback in workers.Walkthrough
vite_prep_html_preview_short.mp4
Rendered HTML preview
Editor remains usable after file switching
Validation
yarn lintpassed.CI=true yarn test runtimeConfig pyodideWorkerUrl PyodideRunner --watchAll=falsepassed: 5 suites / 76 tests.yarn buildpassed with existing webpack size warnings.yarn check-build-artifactspassed: 23 required artifacts found.yarn exec cypress run --config-file cypress.config.mjs --browser chromepassed: 72 tests, 71 passing, 1 pending.To show artifacts inline, enable in settings.