Build the JS render client at packaging time instead of committing it#214
Merged
Conversation
The minified render-client bundles (python/xy/static/{index,standalone}.js)
were committed to git, so every branch that touched the client produced merge
conflicts in a 291 KB generated file. Treat them like the Rust core: a
generated artifact built during packaging, never version-controlled.
- git-ignore the two bundles; the hatch build hook now runs `node js/build.mjs`
for both the wheel and sdist targets (gated on Node via XY_REQUIRE_NODE /
XY_SKIP_NODE, mirroring the cargo switches).
- Include them via `[tool.hatch.build] artifacts` so the git-ignored bundles
ride into both distributions (a build hook's force_include is not honored by
the sdist builder; artifacts is).
- Drop the `node js/build.mjs --check` freshness gate everywhere (nothing
committed to compare against); dist CI jobs set XY_REQUIRE_NODE and let the
hook build, consumers build explicitly. Add Node to install_without_rust and
python_floor (both run bundle-reading tests).
- Keep verify_ci_workflow / verify_local / build.mjs / Makefile / widget error
message / dossier §33 / production-readiness / CLAUDE.md in sync.
End users are unaffected: published wheels and sdists carry the client prebuilt
(no Node needed, incl. `pip install <sdist>`). Only building from a raw clone
now needs Node + `npm ci`, exactly as it already needs Rust for the core.
Greptile SummaryThis PR builds the JavaScript render client during packaging instead of storing generated bundles in Git. The main changes are:
Confidence Score: 5/5This looks safe to merge. Required distribution builds now fail when the client bundles cannot be produced. Active callers no longer invoke the removed read-only check mode. No blocking issues were found in the updated code.
 T-Rex LogsWhat T-Rex did
Important Files Changed
Reviews (3): Last reviewed commit: "Address review: require the render clien..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
CI jobs that install xy from a source checkout (matplotlib_reference, codspeed, docs quality/production) hit a FileNotFoundError once the bundle stopped being committed: HTML export and the reflex_xy adapter link-out both read python/xy/static/*.js, which those jobs never built. Make the build hook self-provisioning instead of patching each job: when the bundle is absent and a source tree is present, run `npm ci` (if needed) then `node js/build.mjs` — the JS analogue of compiling the Rust core from source. It only triggers when the bundle is missing, so installs from a published wheel/sdist stay completely Node-free (they carry it prebuilt). - hatch_build.py: _provision_js returns early if bundles already exist; else builds from js/ source, provisioning vite/tsc via npm ci first. - Pin Node 22 on the source-install jobs that lacked it (matplotlib_reference, codspeed, docs quality) so the hook's build uses a vite-compatible Node. - Fix export.py's stale "npm run build" hint (no such script) to match widget.py. Verified locally: with both the bundle and node_modules removed, `uv build` runs npm ci + build and the wheel/sdist carry the client at the right paths.
…t-repo-b091f0 # Conflicts: # python/xy/static/index.js # python/xy/static/standalone.js
Merging this PR will not alter performance
Comparing 
Footnotes
|
…cript Greptile review of #214: - P1 (incomplete distributions succeed silently): the render client is mandatory in every wheel — verify_wheel.py requires static/*.js in both native and pure wheels — unlike the *optional* native core. So make it required by default instead of opt-in: a from-source wheel/sdist build that can neither find nor build the bundle now fails loudly rather than shipping a broken, client-less distribution. XY_SKIP_NODE=1 is the explicit opt-out (prebuilt-bundle steps, or a deliberately client-less build). Replaces the opt-in XY_REQUIRE_NODE, dropped from the CI/release dist jobs accordingly. - P2 (npm check script mutates): package.json's `check` script still passed the removed `--check` flag, so `npm run check` silently rebuilt in place instead of validating. Nothing invokes it; removed it (build/typecheck remain). Verified locally: with node hidden and no bundle, `uv build` now fails with a clear "render client is required" error by default, and succeeds client-less only under XY_SKIP_NODE=1. verify_ci_workflow + 187 verifier tests pass.
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
1 participant
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.
Why
The minified render-client bundles (
python/xy/static/index.js,standalone.js) were committed to git — two ~291 KB generated files. Any branch that touched the TypeScript client regenerated them, so they were a constant source of merge conflicts. This treats them like the Rust core: a generated artifact built during packaging, never version-controlled.What
hatch_build.py): runsnode js/build.mjsfor both the wheel and sdist targets, gated on Node viaXY_REQUIRE_NODE/XY_SKIP_NODE(mirroring the existingXY_*_CARGOswitches). Registered undertargets.wheel.hooks.customandtargets.sdist.hooks.custom.[tool.hatch.build] artifactsre-includes the git-ignored bundles into both distributions. (A build hook'sbuild_data["force_include"]turned out not to be honored by the sdist builder —artifactsis the mechanism that works for both.)node js/build.mjs --checkfreshness gate everywhere (nothing committed to compare against). Dist jobs setXY_REQUIRE_NODE=1and let the hook build; jobs that read the bundle on disk build it explicitly. Added Node toinstall_without_rustandpython_floor(both run bundle-reading tests).verify_ci_workflow.py,verify_local.py,build.mjs(removed--checkmode),Makefile, thewidget.pyerror message, dossier §33,production-readiness.md,CLAUDE.md.Impact
End users are unaffected — published wheels and sdists carry the client prebuilt, no Node needed (including
pip install <sdist>). The only change: building from a raw git clone now needs Node +npm ci, exactly as it already needs Rust for the core.Validation
verify_sdist.pypassesxy/static/*.jsat the correct path →verify_wheel.py --expect-purepassespip install <sdist>needs no Nodeverify_ci_workflow.py+ 187 verifier/local tests pass;ruff check/formatclean(The native-core test suite wasn't run locally — crates.io is unreachable in the dev sandbox — but that path is exercised by CI.)