fix(compile): #684 — type-only imports must not load as runtime modules#828
Merged
Conversation
`crates/perry/src/commands/compile/collect_modules.rs` iterated every `hir_module.imports` entry through cached_resolve_import and queued them all as runtime modules — ignoring `import.type_only`. The HIR layer at `crates/perry-hir/src/lower.rs:4151` already preserves the type-only annotation per specifier so class metadata can flow into `imported_classes` for method dispatch (the #446 fix), but the collector treated the annotation as informational only and loaded the underlying package anyway. The deterministic Effect repro from the issue: bun add effect@3.21.2 echo 'import {} from "effect"; console.log("ok")' > test.ts perry compile test.ts -o /tmp/out /tmp/out # before: TypeError: Cannot read properties of undefined (reading '_tag') Effect's `Schema.ts` has exactly one `@standard-schema/spec` reference: import type { StandardSchemaV1 } from "@standard-schema/spec" The package ships an empty `var src_exports = {}` at runtime — it's type-only by design. Perry's collector saw the import and queued the package for V8 fallback (only JS module among 363 modules), so any `<StandardSchemaV1>._tag` read inside Effect's compiled code threw V8's exact wording. That uncaught TypeError was the #684 symptom in its current form — the original `(number).slice` message dissolved once intervening Schema/Effect landings advanced init past the old crash point. Fix is one guard at the top of the import-processing loop: skip entries with `type_only: true`. Class-metadata flow is unaffected because HIR lowering already captured it before this point; the collector only governs whether the module's `.o` (or JS bundle) participates in linking and runtime init. Validation: - `import type { Foo } from "./does_not_exist"` now compiles cleanly (was rejected at the resolve step). - Mixed `import { x }` + `import type { T }` from the same file still resolves the value side correctly. - `cargo test --release -p perry-hir -p perry-codegen -p perry` — all green (223 + 29 + 4 + 41 etc., 0 failed). - 7-test class+import smoke set byte-identical to `node --experimental-strip-types`. Follow-up: the Effect repro now advances past the `_tag` crash and exposes a separate link-time gap — `js_readable_stream_*` symbols are unresolved because Effect uses the global `ReadableStream` constructor but `compute_required_features` only triggers `bundled-streams` on a literal `import "streams"`. Auto-enabling the streams feature on global `new ReadableStream()` / `new WritableStream()` / `new TransformStream()` is a separate Effect-e2e (#321) follow-up. Closes #684 (the type-only-runtime-load root cause; the renamed `_tag` symptom this PR was reproducing).
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.
Summary
The module collector iterated every
hir_module.importsentry throughcached_resolve_importand queued them all as runtime modules, ignoringimport.type_only. Soimport type { … } from \"…\"triggered V8 fallback for type-only packages, and any access to the (non-existent) named imports during init threwTypeError.Deterministic repro
Effect's
Schema.tshas exactly one@standard-schema/specreference —import type { StandardSchemaV1 } from \"@standard-schema/spec\". The package ships an emptyvar src_exports = {}at runtime (type-only by design). The collector saw the import and routed the package through V8 (only JS module among 363), so any<StandardSchemaV1>._tagread inside Effect's compiled code threw V8's exact wording. That uncaught TypeError is the #684 symptom in its current form — the original(number).slicemessage dissolved once intervening landings advanced Effect's init past the old crash point.Fix
One guard at the top of the import-processing loop in
collect_modules.rs— skip entries withtype_only: true. Class-metadata flow is unaffected: the HIR layer atlower.rs:4151already captures type-only specifiers intoimported_classesfor method dispatch (#446 path). The collector only governs whether the module's.o(or JS bundle) participates in linking and runtime init.Test plan
import type { Foo } from \"./does_not_exist\"now compiles cleanly (was rejected at the resolve step).import { x }+import type { T }from the same file still resolves the value side correctly.cargo test --release -p perry-hir -p perry-codegen -p perry— all green (223 + 29 + 4 + 41 + 1 + 40 + 10 + 6, 0 failed).node --experimental-strip-types.cargo fmt --all -- --check— clean.Follow-up
The Effect repro now advances past the
_tagcrash and exposes a separate link-time gap:js_readable_stream_*symbols are unresolved because Effect uses the globalReadableStreamconstructor (no literalimport \"streams\"), andcompute_required_featuresonly triggersbundled-streamson the named import. Auto-enabling the streams feature on globalnew ReadableStream()/new WritableStream()/new TransformStream()is a separate Effect-e2e (#321) follow-up.Closes #684.