refactor(core): move report template out of core js output#2522
Open
EAGzzyCSL wants to merge 1 commit into
Open
refactor(core): move report template out of core js output#2522EAGzzyCSL wants to merge 1 commit into
EAGzzyCSL wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR decouples @midscene/core from the apps/report build by removing build-time HTML injection into core’s JS output, instead treating the report template as a static asset (with an optional global injection path for bundled targets). This reduces build-order coupling and avoids recursive template growth caused by repeated “magic string” injection.
Changes:
- Added build utilities to (a) sync
apps/report/dist/index.htmlintopackages/core/dist/report-template/index.htmland (b) inject the template into bundle targets by replacing@midscene/coreimports with wrapper modules. - Updated
@midscene/coreruntime to resolve the report template fromglobalThis, dev template path, or the staticdist/report-template/index.htmlfile (and to error when missing in browser bundles). - Removed the old “REPLACE_ME_WITH_REPORT_HTML” injection flow and updated configs/docs/tests/packaging checks to align with the new mechanism.
Reviewed changes
Copilot reviewed 30 out of 31 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/rsbuild-utils.ts | Adds wrapper-module replacement plugin for bundling and a post-build sync plugin for the report template asset. |
| pnpm-lock.yaml | Updates workspace importer devDependencies to include @midscene/report where needed for build ordering. |
| packages/web-bridge-mcp/rslib.config.ts | Switches from the legacy injection plugin to the new core wrapper replacement plugin. |
| packages/shared/src/mcp/inject-report-html-plugin.ts | Removes the legacy “magic string” injector that rewrote built JS outputs. |
| packages/shared/src/mcp/index.ts | Stops exporting the removed legacy injection plugin. |
| packages/playground/src/adapters/local-execution.ts | Removes legacy placeholder filtering when reading report HTML from an agent. |
| packages/mcp/rslib.config.ts | Adds the new core wrapper replacement plugin to ensure bundled outputs inject the template. |
| packages/ios-mcp/rslib.config.ts | Replaces legacy injection with the new core wrapper replacement plugin. |
| packages/harmony-mcp/rslib.config.ts | Replaces legacy injection with the new core wrapper replacement plugin. |
| packages/core/rslib.config.ts | Removes the old post-build JS scanning/injection logic from the core build. |
| packages/core/src/utils.ts | Changes getReportTpl() to read from a global injection first, then dev path, then the static template asset (or throw in browser). |
| packages/core/tests/unit-test/utils.test.ts | Adds global template setup/teardown to keep unit tests independent of the on-disk template asset. |
| packages/core/tests/unit-test/report.test.ts | Adds global template setup/teardown for report-related unit tests. |
| packages/computer-mcp/rslib.config.ts | Replaces legacy injection with the new core wrapper replacement plugin. |
| packages/android-mcp/rslib.config.ts | Replaces legacy injection with the new core wrapper replacement plugin. |
| CONTRIBUTING.md | Updates contributor docs from placeholder-injection troubleshooting to “missing report template” guidance. |
| apps/studio/tests/package-electron.test.mjs | Updates packaging tests to detect template injection rather than placeholder strings. |
| apps/studio/scripts/package-electron.mjs | Updates packaging validation to require injected template content in runtime output and removes core placeholder checks. |
| apps/studio/rsbuild.config.ts | Adds the core wrapper replacement plugin to inject the template into the studio bundle. |
| apps/studio/package.json | Adds @midscene/report as a devDependency for build ordering. |
| apps/report/src/components/playground/index.tsx | Avoids collecting/storing report HTML when downloading is disabled. |
| apps/report/scripts/inject-report-template.mjs | Removes the legacy script that rewrote core dist JS output. |
| apps/report/rsbuild.config.ts | Replaces JS injection logic with a post-build sync of the template into core’s dist asset path. |
| apps/report/package.json | Declares NX build outputs to include the synced core template asset directory. |
| apps/playground/rsbuild.config.ts | Adds the core wrapper replacement plugin to inject the template into the playground bundle. |
| apps/playground/package.json | Adds @midscene/report as a devDependency for build ordering. |
| apps/computer-playground/rsbuild.config.ts | Adds the core wrapper replacement plugin to inject the template into the bundle. |
| apps/computer-playground/package.json | Adds @midscene/report as a devDependency for build ordering. |
| apps/android-playground/rsbuild.config.ts | Adds the core wrapper replacement plugin to inject the template into the bundle. |
| apps/android-playground/package.json | Adds @midscene/report as a devDependency for build ordering. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
EAGzzyCSL
force-pushed
the
zzy/report-template-alt
branch
from
18e7fb8 to
4ff4721
Compare
Deploying midscene with
|
| Latest commit: |
66a69b5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5b5d509e.midscene.pages.dev |
| Branch Preview URL: | https://zzy-report-template-alt.midscene.pages.dev |
EAGzzyCSL
force-pushed
the
zzy/report-template-alt
branch
4 times, most recently
from
0a96d81 to
88d12e7
Compare
EAGzzyCSL
force-pushed
the
zzy/report-template-alt
branch
from
88d12e7 to
66a69b5
Compare
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.
Background
Previously, the
coreandreportbuilds were tightly coupled.On one side,
apps/reportdepends on@midscene/corebecause the report UI reuses core types, dump structures, and helper logic.On the other side, the built output of
@midscene/coreneeded to inline the report HTML template. In other words, thereportbuild depended oncore, but the final usablecoreoutput also depended onreportbuilding first and writing the HTML back into core.This made the build order hard to reason about:
corehad to be built beforereport, but afterreportfinished building, it had to modifycore distagain. The old approach handled this by scanning JS files undercore dist, finding magic strings such asREPLACE_ME_WITH_REPORT_HTML, and replacing them with the report HTML string.This mechanism was also prone to repeated injection. The report HTML is large, and the report app may build against an already-built
core dist. If thatcore distalready contains an injected report HTML template, the nextapps/reportbuild may bundle that template back into the new report HTML. Writing the new report HTML back into core then creates recursive growth, where the HTML contains the previous HTML and keeps growing in subsequent builds.Current Approach
The new approach avoids this build-time coupling and separates responsibilities.
apps/reportis only responsible for generating the report template. After it builds, it syncsapps/report/dist/index.htmlinto@midscene/coreas a static asset:@midscene/coreno longer inlines the report template into its JS output, and it no longer relies on magic string replacement. At runtime, when core needs to generate a report, it reads this template file from its own package.This makes the responsibility boundary clearer:
apps/reportgenerates the report template@midscene/coreconsumes the template at runtimecore distFor cases where
@midscene/coreis bundled into a single file or a frontend bundle, the static template file may not be included in the final output automatically. For those cases, a global injection escape hatch is provided:Core reads this global variable first. If the template has already been injected in a bundled runtime, core does not need to read it from the file system.
Internal bundle targets in this repository use a build plugin for this. The plugin replaces
@midscene/coreentries with wrapper modules, injects the report template first, and then re-exports the original core entry. This puts template injection into the bundler module graph, so we do not need to rewrite final JS output or return to the old magic string replacement approach.