Claude/js build api design 7luaw8 - #249
Merged
Merged
Conversation
`moon build --target js` failed in `src/cmd/mtsc/watch.mbt`: `@async_fs.mtime` does not exist on the JS backend, because `moonbitlang/async/fs` states in its own `unimplemented.mbt` that the package "currently does not support JavaScript backend" and restricts every other source file to `native` / `wasm`. The CLI is a native executable, so it now declares `supported_targets = "all-js"` and that build skips it instead of failing inside it. A per-target `#cfg` arm for the one call is not enough: the native arm is `async` only by virtue of that call, so a sync JS arm makes the `async` on `mtsc_watch_stamp` AND `mtsc_watch_stamps` useless and the workaround spreads a function at a time up the call chain. Nothing was lost that anything used — in this repo `--target js` only ever built generated example packages, and no harness built the CLI for JS, which is why a whole-module JS build was broken unnoticed. `moon check --target js` still covers all 42 packages, so nothing stopped being checked. The type checker reaches JavaScript through `src/mtsc` instead, which is now a library rather than a string ABI. `checkModuleGraph` made the caller pre-resolve the whole program before it could type-check one file; resolution moves in and the IO moves out, behind an `MtscHost` trait. That is TypeScript's split, where `LanguageServiceHost` supplies file access and the service resolves, and the API is named after `ts.LanguageService` so an existing integration reads the same: getProgramDiagnostics / getSemanticDiagnostics / getSyntacticDiagnostics / getProgramFileNames / resolveModuleName, returning structured diagnostics rather than a joined string. A trait rather than a struct of closures because MoonBit requires an `impl` to supply every method, so a new host cannot inherit a default that fails open. Synchronous, and therefore NOT shared with `src/parser`'s resolver, which is `async` throughout — a shared interface today would fit neither consumer. `collect_parsed_graph_issues` is extracted verbatim from `collect_module_graph_issues`, so a host-loaded program is checked by the same code as a pre-resolved one and cannot drift from it. `checkModuleGraph` is unchanged and still has its consumers. `start` / `length` are absent from a diagnostic: `ExprIssue` and `ParseError` carry no offsets, and `ts.Diagnostic` declares both optional for exactly this case. The checker's breadcrumb ships as `context` instead. Scope kept deliberate: relative specifiers only (bare ones report `isExternal` rather than a failure), and no completions / quick info / definitions / rename, all of which need a position. Verified: `moon build --target js` clean from scratch, `moon fmt --check`, `moon check --deny-warn` (native gate), `moon check --deny-warn --target js src/mtsc` (the native check cannot see the js-only FFI files at all, so this is a new gate), `moon test --target native` 3066/0, 35 new native cases, 22 Node cases, and the shipped `.d.ts` under `tsc --strict`. The Node harness was mutation-proven rather than trusted for passing first time: dropping the `.js` -> `.ts` remap, swallowing parse errors, or removing the `getScriptSnapshot` fallback each fail it. One of those was a real bug in the first draft — the loader looked for parse failures among the files that had parsed, so no syntax error could ever be reported. The no-span assertion needed the same treatment: spelled `@ts-expect-error` over `d.start` it passed whether or not the field existed, so it is a `keyof` assertion now, which fails when the field is added. Two pre-existing `unused_package` warnings remain on `moon check --target js` for `src/`, whose `moonbitlang/async/fs` import is native-only. moon has no per-target import syntax, and the alternatives are suppressing the warning or declaring the root package native-only, which would be untrue. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xb1GadNQusxErePTkw1zGn
The previous commit fixed `moon build --target js` by declaring
`supported_targets = "all-js"` on the CLI, so the build skipped it. That
fixed the build by trading the capability away, and the trade was
unnecessary: only ONE symbol was missing (`@async_fs.mtime`) and the
rest of the CLI compiled for `js` untouched.
`moonbitlang/async/fs` says in its own `unimplemented.mbt` that it "does
not support JavaScript backend" and restricts every source file but that
one to `native` / `wasm`. The per-target arm is cheap because the native
arm is `async` only by virtue of that one call, so a sync JS arm makes
the `async` on `mtsc_watch_stamps` useless — fixed with one
`#warnings("-unused_async")` at that declaration rather than a
per-target duplicate of a six-line loop. `node:fs/promises` would keep
it async and match `mizchi/x/fs`'s own JS backend, but awaiting a JS
promise needs `moonbitlang/async/js_async`, unused on native, and native
`--deny-warn` is the gate.
The unused-import problem those dependencies create needed no
suppression: `@async_fs.unimplemented` is the one symbol the package
exposes on `js`, declared for exactly this, so naming it from the live
`js` arm in each of the two packages makes the import genuinely
referenced everywhere. `moon check --deny-warn --target js` is now clean
for the WHOLE module and `just check-js` widened from `src/mtsc` to all
of it. No `warnings = "-0029"` anywhere — a package-wide suppression
also hides a dead import, on every target.
Two bugs were found by RUNNING it, neither reachable by building.
`@env.args()` does not have the same shape on every backend:
`moonbitlang/core/env` returns `process.argv` verbatim on `js`, which
has two leading entries where the native runtime's argv has one.
`driver.mbt` reads that shape at six places, so under Node the CLI took
its own 19 MB bundle as an input file and reported
`ParseError("Unexpected token: Gt")` against itself — `mtsc ok.ts
--noEmit` on a clean file exited 1. Normalized once in `mtsc_argv`
rather than patched at six sites.
The second was silent. The mtime probe first read `require("node:fs")`,
and the bundle is a `.js` IIFE under a `package.json` saying `"type":
"module"`, so Node loads it as ESM where `require` is undefined — which
the body's own `catch` turned into "no stamp" for every file. "No stamp"
compares equal to the previous "no stamp", so `--watch` would have
polled forever and never rebuilt, looking exactly like a watcher with
nothing to do. `process.getBuiltinModule` is the sync builtin accessor
that works from either module system, with `require` kept as the
CommonJS fallback.
`just verify-cli-node` is a differential against the native binary —
same source, a different backend and a different runtime for every
syscall — over 19 cases plus a `--watch` round trip on both backends,
comparing stdout, the exit code, and for the emit cases the file
written. Three cases cover the `bridge` / `pkg` verbs, which route
through `mizchi/ts` and its own per-target split
(`publish_staged_bridge_file`), so that arm does not ship unexercised.
Mutation-proven rather than trusted for passing first time: reverting
the argv normalization fails eight cases, reverting the mtime probe
fails the watch round trip with the diagnostic written for it, and
dropping the JS publish write fails the bridge-package case on all four
emitted files. Exactly one case is allowed to diverge and says why — a
missing entry file, where `mizchi/x/fs` relays the OS error text and the
two backends word it differently; it asserts what must hold for both,
still requires the exit codes to match, and FAILS if the outputs ever
become identical so the allowance cannot outlive its reason.
Verified: `moon fmt --check`, `moon check --deny-warn` (native gate),
`moon check --deny-warn --target js` (whole module, clean), `moon test
--target native` 3066/0, 21/21 in the new harness, 22/22 in the
language-service harness, and the native CLI unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xb1GadNQusxErePTkw1zGn
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
2 participants
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.
No description provided.