feat: replace legacy web explorer preview with Bevy Web - #1502
Conversation
Deploying js-sdk-toolchain with
|
| Latest commit: |
be41563
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c8d1a08d.js-sdk-toolchain.pages.dev |
| Branch Preview URL: | https://feat-replace-web-explorer-wi.js-sdk-toolchain.pages.dev |
Test this pull request
|
The @dcl/explorer unity web build was a 244MB dependency frozen at an Aug 2024 snapshot, downloaded on every scene install only to serve the deprecated --web-explorer preview. Drop it and the preview-server routes that served it; --web/--bevy-web (now pointing at decentraland.org instead of .zone) covers browser preview via the hosted Bevy Web client, with the local server acting as its realm. --web3 and --no-debug remain declared as no-ops so existing start scripts keep working, and the CLI prints Chrome Local Network Access guidance since decentraland.org needs the 'Apps on device' permission to reach localhost.
2d718be to
d74ad02
Compare
|
Just one comment: hot reload doesn't seem to be working |
decentraland-bot
left a comment
decentraland-bot
left a comment
There was a problem hiding this comment.
Review: feat: replace legacy web explorer preview with Bevy Web
Branch: feat/replace-web-explorer-with-bevy-web → main
Files changed: 7 (+42 −167) — net deletion, nice cleanup.
Clean, well-scoped PR that removes the 244 MB Unity web build (@dcl/explorer) and makes Bevy Web the browser preview path. The PR body is thorough and the test updates are correct. One blocking issue with a removed flag that still has downstream consumers.
P1 — Blocks merge
[Consumer breakage] --web-explorer removed from the arg parser will break downstream consumers.
The arg library throws on unknown flags. Removing '--web-explorer': Boolean from declareArgs means any script passing --web-explorer will crash on the next SDK release. At minimum:
decentraland/godot-explorer—scripts/bench/launch_devices.shpasses--web-explorerdecentraland/docs—creator/sdk7/getting-started/preview-scene.mddocuments--web-exploreras a valid flag
Fix: Keep '--web-explorer': Boolean in the args declaration (accepted but ignored), document it as deprecated in the help text like --web3 and --no-debug, and open a follow-up issue/PR on decentraland/docs to update the docs.
P2 — Minor
-
[Dead code]
--explorer-alphais still declared in args but is now effectively a no-op. TheexplorerAlphavariable is simply!bevyWeb— the--explorer-alphaflag value is never read. Consider deprecating it in the help text (same pattern as--web3/--no-debug) or removing the declaration if no consumers pass it. -
[Naming]
explorerAlphavariable is now misleading. It's just!bevyWeb— the name suggests the old alpha explorer distinction that no longer exists. A rename to something likedesktopExplorer(or inlining!bevyWeb) would improve readability. -
[Pre-existing bug] chokidar
unlinkhandler has wrong callback signature..on('unlink', (_: unknown, file: string) => { removeModel(sceneId, file) })
Chokidar's
unlinkevent passes a single argument(path), not(event, path). Here_captures the path andfileis alwaysundefined, makingremoveModela no-op (it exits early onisGLTFModel(undefined)). This bug existed before this PR but is now executed unconditionally. Not blocking, but worth fixing while touching this file. -
[Docs follow-up]
decentraland/docsneeds a companion update to replace--web-explorerreferences with--web/--bevy-web.
Notes
- @eordano's hot reload comment: The code changes look correct for hot reload — both protobuf and legacy WS messages are now sent unconditionally, which is strictly more coverage than before. The hot reload issue is likely related to Chrome's Local Network Access policy blocking WebSocket connections from
decentraland.orgtolocalhost(the PR already adds guidance about this). Worth confirming whether hot reload works once the permission is granted. - Security: No new vulnerabilities introduced. The
/feature-flags/:fileproxy is pre-existing and scoped tofeature-flags.decentraland.zone. No secrets, no injection vectors. @dcl/explorerremoval from@dcl/sdk: Safe — downstream lockfiles pin their own copy, and theserveStaticconsumer is fully removed.- CI: All checks still pending at review time (build, test, lint, docs).
Reviewed by Jarvis 🤖 · Requested by Gonzalo Pombo (<@U02EUDGAM6W>) via Slack
| @@ -53,7 +53,6 @@ export const args = declareArgs({ | |||
| '--skip-build': Boolean, | |||
| '--data-layer': Boolean, | |||
| '--explorer-alpha': Boolean, | |||
There was a problem hiding this comment.
[P1] --web-explorer was removed from declareArgs, but decentraland/godot-explorer (scripts/bench/launch_devices.sh) and decentraland/docs still reference it. The arg library throws on unknown flags, so this will crash those scripts on the next SDK release.
Keep it as a deprecated no-op (same pattern as --web3 and --no-debug):
| '--explorer-alpha': Boolean, | |
| '--explorer-alpha': Boolean, | |
| '--web-explorer': Boolean, // deprecated no-op, kept for backwards compatibility |
| @@ -36,17 +33,15 @@ export async function wireFileWatcherToWebSockets( | |||
| cwd: projectRoot | |||
| }) | |||
| .on('unlink', (_: unknown, file: string) => { | |||
There was a problem hiding this comment.
[P2] Pre-existing bug: chokidar unlink fires with a single (path) argument, not (event, path). Here _ captures the path and file is always undefined, making removeModel a no-op.
Suggested fix:
| .on('unlink', (_: unknown, file: string) => { | |
| .on('unlink', (file: string) => { | |
| removeModel(sceneId, file) | |
| }) |
|
Re: review P1 ( P2.1 ( |
decentraland-bot
left a comment
decentraland-bot
left a comment
There was a problem hiding this comment.
Re-review: feat: replace legacy web explorer preview with Bevy Web
Re-reviewed based on the author's feedback on the previous round. No code changes since the last review (latest commit is a merge from main).
Previous findings — updated
-
P1 (Withdrawn. Author's rationale accepted: the old web explorer no longer exists, so a hard error on the next release intentionally surfaces the migration. Keeping the flag as a no-op would hide that scripts are targeting a dead client. Docs update is covered by decentraland/docs#150.--web-explorerremoval) -
P2.1 (Withdrawn. Creator Hub (and possibly other processes) still launch the CLI with--explorer-alphadead code)--explorer-alpha, so the declaration must stay to avoid breaking those flows.
Remaining P2 (non-blocking)
- [Pre-existing bug] chokidar
unlinkhandler has wrong callback signature —file-watch-notifier.ts:35: chokidarunlinkfires with(path), but the handler destructures(_: unknown, file: string), sofileis alwaysundefinedandremoveModelis a no-op. Not introduced by this PR, but worth fixing in a follow-up since it's now executed unconditionally.
Verdict
Approved. Clean, well-scoped removal that drops 244 MB of dead weight. The breaking change is intentional and well-documented. No security issues. CI is still running at review time.
Reviewed by Jarvis 🤖 · Requested by Gonzalo Pombo (<@U02EUDGAM6W>) via Slack
Chokidar's unlink event passes a single (path) argument, not (event, path). The previous signature (_ : unknown, file: string) captured the path in _ and left file undefined, making removeModel a no-op for deleted GLB/GLTF files.
Rebased onto main (0f012e0, "replace legacy web explorer preview with Bevy Web" #1502). The branch's 38 commits are collapsed into one: they included an entire abgen *sidecar* implementation that later commits deleted and replaced with the in-process addon, so replaying them would have meant resolving conflicts in code that does not survive. sdk-commands start now converts through @dcl/abgen-node — a function call, not a spawned process — and serves the bundles from the preview server itself, so there is one origin and no port to coordinate. Conversion is keyed on a sha256 of the publishable files. Identical content means the same key, so a request for work already in flight joins it rather than starting a second, and a run publishes only if its key is still the one most recently asked for. Results publish by atomic rename out of a private staging directory, with an advisory lock and an age-gated sweep, so two previews of one project cannot interleave bundle files into a directory a reader is about to serve. Two conflicts with #1502, both resolved in main's favour: - wireFileWatcherToWebSockets: main now sends updateScene unconditionally, because the legacy JSON protocol is the only one Bevy and Godot understand. This branch had gated it behind a desktopClient flag, which would have regressed Bevy Web, so the flag is dropped and only the onProjectChanged invalidation is layered on. - its 'unlink' handler: main's (file) signature is correct; this branch read (_, file), which left file undefined, since chokidar emits the path as the first argument. 52/52 green across the four start-command suites.
4 participants
Summary
Removes the unity-renderer web preview and makes Bevy Web the browser preview path:
@dcl/explorerdependency from@dcl/sdk— a 244 MB unity web build pinned to an Aug 2024 snapshot, downloaded on every scene install.--web-explorerand the preview-server routes that served the web client (serveStatic:preview.html,/@/explorer/*,/default-profile/*, connect images). The/feature-flagsproxy is kept (client-agnostic).__LEGACY__updateScene, JSONSCENE_UPDATE) — Bevy and Godot explorers only understand this protocol, so it is now broadcast unconditionally alongside the protobufWsSceneMessage(previously the protobuf path only fired for desktop/bevy-web clients). Removal is tracked by the client migrations: Preview hot-reload: migrate from legacy JSON SCENE_UPDATE to protobuf WsSceneMessage bevy-explorer#1020, Preview hot-reload: migrate from legacy JSON SCENE_UPDATE to protobuf WsSceneMessage godot-explorer#2600.--webas an alias of--bevy-web, and points the Bevy Web URL atdecentraland.org/bevy-web(wasdecentraland.zone).--web3and--no-debugstay declared as documented no-ops so existingnpm start -- --web3scripts don't hard-error.start --webprints Chrome 142+ Local Network Access guidance ("Apps on device" permission), since the hosted site must be allowed to reach localhost. Client-side fix: fix: annotate loopback fetches for Chrome Local Network Access bevy-explorer#1017.What could break
http://localhost:PORT?SCENE_DEBUG_PANEL...) — the local server no longer serves a web client at/; it remains the realm/content server.@dcl/sdkversions keep working — they pin their own@dcl/explorercopy.@dcl/explorerentry still visible insdk-commands/package-lock.jsonbelongs to the published@dcl/sdk@7.17.0dev-dep tree and clears on the next release.How to test
make build, then in a scene:sdk-commands start --web→ browser openshttps://decentraland.org/bevy-web/?preview=true&realm=http://127.0.0.1:PORT; allow the Chrome "Apps on device" prompt; scene loads and hot-reloads on file changes.sdk-commands start(no flag) → desktop Explorer deeplink flow unchanged.npx jest --testPathPattern='kernel-and-renderer-version|test/sdk-commands/commands/start'→ passes (28 tests).tsc --noEmitinsdk-commandspasses.@dcl/explorer(244 MB smaller).