Repository navigation
fix(client): ship lib/client.js as the DSH browser bundle, not tsc output - #11
JosephTian876 wants to merge 1 commit into
Conversation
2a66708 to
46d60ac
Compare
|
Rebased onto current The conflicts were all in files this PR replaces:
Verified on the rebased commit:
Meanwhile the defect is still live. Re-measured across published and in-tree artifacts:
It has now been reported independently in #12 (host Nothing about the fix itself changed in the rebase — it just needed to be mergeable again. Happy to adjust if you'd like it split or scoped differently. |
A local (`uses: ./`) action needs a checkout first, so the workflow checks out and then calls the action with fail-on: error. It doubles as the action's integration test: the next pull request here exercises the composite wiring, the diff fetch, the analysis and the review posting for real. Also worth recording: the action's two `run:` scripts were executed for real locally through Git Bash against a live pull request (PerryLink/dsh-github#11, 10 files, +762/-301). The empty-pull-request guard exits 1, the diff fetch returns 85 KB, the analysis reports 0 error / 3 warning / 2 note, GITHUB_OUTPUT receives errors/warnings/notes plus the findings heredoc, and the token appears in neither step's output. The one warning on that pull request is a true positive: `new Function('window', 'require', 'module', 'exports', code)`.
|
谢谢提交. 已复核: 产物 sha256 一致, main 仍复现 (ModuleLoader 为 0), 冲突仅 CHANGELOG.md. 仲裁取这条: external 共享运行时更稳, 不带第二份单例. 席位让 #14 收窄, settingsScope 我来补. 要我接手也行. |
…tput
`exports["./client"].default` published the Node-side `tsc` compile of
`src/client.ts` — top-level `import`/`export` — but `@deepseek-ai/dsh-client-modules`
does not import that file as a module. It installs it with
`document.createElement('script')` and expects the classic script to hand its
factory to `window.__ModuleLoader__.load({ id, factory })`. The browser throws
`SyntaxError: Cannot use import statement outside a module` at the file, and
because the host concatenates several packages into one combo script per batch,
that parse error aborts the whole batch: every client entry in it stays
unactivated and the UI reports **"Failed to load plugins"** naming this package.
Reproduced in a clean headless Edge against a local `0.1.7-alpha.2` host before
the change (the SyntaxError lands on the batch URL, then
`web boot: 1 entry did not activate / @perrylink/dsh-github: import failed`), and
absent after it: every served batch parses as a classic script, the page boots
with zero window errors, and the card mounts.
`pnpm build` now emits the artifact in the loader's shape:
window.__ModuleLoader__.load({ id: "@perrylink/dsh-github", factory: (require) => { ... } })
with `react` and `@deepseek-ai/dsh-client-ui-primitives` left as `require(...)`
for the shell's module table, and everything else inlined. A bare specifier in
the shared-runtime families (`@deepseek-ai/cordis`, `@deepseek-ai/dsh-client-*`)
that is not a table row fails the build instead of inlining a second copy of a
singleton the shell already holds.
- `scripts/client-bundle.mjs` — the artifact contract: module-table externals,
the registration wrapper, and the checks that prove a produced file has it.
- `scripts/build-client.mjs` — the build step, also exposed as `pnpm build:client`.
- `scripts/prepare.mjs` — requires `typescript` **and** `esbuild` before it
compiles anything, because `tsc` alone overwrites a good committed
`lib/client.js` with plain ESM; the git-install fallback (no devDependencies)
now accepts committed artifacts only when they are a real bundle.
- `scripts/verify-artifacts.mjs` and `test/client-bundle.test.ts` — execute the
shipped file the way the shell does, as a classic script against a stub
`window.__ModuleLoader__`, so a leftover `import` fails with the browser's own
`SyntaxError` rather than passing a text scan. `node --check` cannot see this:
the package is `"type": "module"`, which makes that parse the file as ESM.
`esbuild` joins `devDependencies`. The lockfile gains only its importer row —
the package was already resolved there as a vitest dependency, and
`pnpm-workspace.yaml` already allowlists its postinstall.
`lib/client.js` and `lib/client.js.map` are rebuilt and committed, as this
repository already does for every artifact. The cordis plugin face (`apply`,
`inject`) and the published `./client` types are unchanged.
46d60ac to
6ad589e
Compare
|
重写后已推到 1. Rebase基点 rebase 后 2. 产物：与上轮你复核过的那份逐字节相同这点是对你 sha256 复核的回应——重写没有让它失效：
3. 独立验收：把产物当 classic script 实跑CI 那 4 项暂时跑不起来（见第 5 点），所以我自己做了同位置的对照实验 —— 用
另外做了一次 batch 拼接验证（把产物和邻居 bundle 拼进同一个 classic script）：修复后文件在前或后都无错、两个 plugin 都注册成功；对照组则 顺带核对了两个 externals 确实在宿主模块表里： 本地跑完 CI 全部命令，退出码 0： 4. 你的仲裁我都照做了，顺带一个提醒「external 共享运行时更稳，不带第二份单例」—— 产物里只
5. 唯一需要你动一下的地方：CI 在等你点批准push 之后三个 workflow 起来了，但状态是 上轮 10-05 那次能跑出结果，是因为你亲手批过（那批 run 的 顺带说明：上一轮那个 6. 一个我在自己机器上发现的低危问题，不影响本 PRWindows + 根治办法是加一条 7. 我明确没有验证的部分
|
|
补充一个 0.2.0-rc.2 宿主上的验证数据点，以及 0.7.20 的产物核对结果，供合并决策参考。 0.7.20 仍未包含本 PR 的修复对比 npm 上的
也就是说，从 0.7.13 到 0.7.20 的所有已发布版本，客户端产物都还是 tsc 直出的裸 ESM。 本 PR 的产物在 0.2.0-rc.2 上实测可用
作为对照：同一台机器上从市场安装 0.7.19 时，直接导致 renderer 引导失败、应用进入恢复模式（ 这个数据点说明本 PR 的修复对 0.2.0-rc.2 同样有效，应该可以直接合并。 |
3 participants
What this fixes
@perrylink/dsh-github@0.7.12publishesexports["./client"].default—lib/client.js— as the Node-sidetsccompile ofsrc/client.ts: ordinary ESM with top-levelimport/export.The DSH web shell does not import that file as a module.
@deepseek-ai/dsh-client-modulesinstalls it withdocument.createElement('script')and expects a classic script that hands its factory towindow.__ModuleLoader__.load({ id, factory }). The browser therefore throws:and because the host concatenates several packages into one combo script per batch, that parse error aborts the whole batch. Every client entry in the batch stays unactivated and the UI reports:
Reproduction
Clean headless Edge against a local DSH
0.1.7-alpha.2, same profile, only the artifact swapped:SyntaxError: Cannot use import statement outside a module×3The bytes the host serves for this package are verified byte-identical to the built artifact (modulo the host's own
;\nseparator and rewrittensourceMappingURL), and the host's composed source map still carries this bundle's mappings.The change
pnpm buildnow emits the browser half in the loader's shape:reactand@deepseek-ai/dsh-client-ui-primitivesstay barerequire(...)calls — they are rows of the shell's frozen module table — and everything else is inlined.@deepseek-ai/cordis,@deepseek-ai/dsh-client-*) that is not a table row now fails the build with an actionable message, instead of inlining a second copy of a singleton the shell already holds.scripts/client-bundle.mjsscripts/build-client.mjspnpm build:clientscripts/prepare.mjstypescriptandesbuildbefore it compiles anything —tscalone overwrites a good committedlib/client.jswith plain ESM — and the no-devDependencies git-install fallback now accepts committed artifacts only when they are a real bundlescripts/verify-artifacts.mjs,test/client-bundle.test.tswindow.__ModuleLoader__node --checkcannot catch this: the package is"type": "module", so Node parses the file as ESM and theimportis legal. Executing it as a classic script reproduces the browser's ownSyntaxError— which is what the new guard does, and it fails loudly when the 0.7.12 artifact is put back.esbuildjoinsdevDependencies. The lockfile gains only its importer row: the package was already resolved there as a vitest dependency, andpnpm-workspace.yamlalready allowlists its postinstall, so no workspace-config change is needed.lib/client.jsandlib/client.js.mapare rebuilt and committed, as this repository already does for every artifact. The cordis plugin face (apply,inject) and the published./clienttypes are unchanged.Verification
pnpm install --frozen-lockfile,typecheck,typecheck:ci,test(189 passed, 3 skipped),test:coverage,lint,build,verify:self-contained,verify:artifacts,pack,check:readmes— all green.prepare+buildleavesgit statusclean, so the committed artifact is exactly what the build produces.verify:artifacts,test/client-bundle.test.ts, andprepare(git-install fallback) all fail with the browser's ownSyntaxError.Follow-up (not in this PR)
lib/types/client.d.ts— the hand-authored published./clienttype face — still declares the removedsettingsScopemember thatsrc/client.tsdropped in 0.7.12.