fix(dynamic): map unmappable function params/returns and type parameters to JSVAL - #215
Closed
techfreaque wants to merge 1 commit into
Closed
fix(dynamic): map unmappable function params/returns and type parameters to JSVAL#215techfreaque wants to merge 1 commit into
techfreaque wants to merge 1 commit into
Conversation
…mode
Five gaps where types that can't compile statically caused SC errors or
spurious mapping failures in --dynamic mode, where npm-origin values
are expected to flow as island handles.
1. **TypeParameter constraint fallback** (`types.ts`): an unresolved
TypeParameter (no binding, or binding returned null) now falls back
to its BASE CONSTRAINT. If the constraint maps to JSVAL (e.g.
`TSchema extends z.ZodTypeAny`) the parameter itself maps to JSVAL —
the actual value will always be an npm handle at runtime. Avoids
spurious SC2008 on `TConfig & { ... }` intersections where `TConfig`
is constrained to a Zod/npm type. Requires a new
`CheckerFacade.getBaseConstraintOfType` wrapper (`checker.ts`).
2. **Function param JSVAL fallback** (`types.ts`): a single-signature
function whose parameter type can't compile statically (complex Zod
schemas, React prop types) now maps the parameter to JSVAL instead of
failing the whole function mapping. The function is still callable
from static code; callers pass island values.
3. **Function return JSVAL fallback** (`types.ts`): a function whose
return type can't compile statically (e.g. `(): JSX.Element` —
ReactElement has any-typed fields) now maps as `func(...) → JSVAL`
instead of returning null, making the function unusable. The return
value rides the island.
4. **Multi-signature → JSVAL** (`types.ts`): overloaded functions
(multiple call signatures) in --dynamic mode now map to JSVAL instead
of being unmappable. Carve-out: stdlib functions from `@types/node`
(e.g. `spawnSync`, `readFileSync`) have static lowerings and must NOT
map to JSVAL — preserved by checking `isStdlibFile` on all
declarations.
5. **`unknown` → DYN in dynFallbackType** (`lowerer.ts`): in --dynamic
mode, `unknown`-typed catch bindings and parameters now fall through
to the checked-dynamic `dyn` representation (same as `any`), allowing
`instanceof Error`, `typeof`, and dyn-compatible operations without
SC2020 fences. Previously `unknown` fell through to null, causing the
whole expression to be unfenced.
6. **Any function is liftable in --dynamic mode** (`lowerer.ts`): the
`jsvalLiftable` check for `func` types now short-circuits to `true`
in --dynamic mode instead of calling `canMarshalTypedFuncIntoIsland`.
The runtime wraps the function as a host callback; type mismatches are
the caller's problem (always was true for npm island code).
`packages/compiler` builds clean (`tsc -p tsconfig.json`, 0 errors).
Contributor
|
@techfreaque is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
Author
|
Superseded by #221 which removes the unrelated MIDI type additions. The dynamic-mode JSVAL fallbacks are identical; this PR is closed to keep the review surface focused. |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| import { arrayOf, BOOL, bytesOf, canConvertToDyn, CHILD_T, DATE_T, DYN, F64, funcOf, isSupportedArrayElem, isSupportedIndexValue, isSupportedMapKey, isSupportedMapValue, isSupportedSetElem, isUnitType, JSVAL, mapOf, NULL_T, PROCSTREAM_T, RUNTIME_EMITTER_CLASS, RUNTIME_ERROR_CLASSES, RUNTIME_STREAM_CLASSES, setOf, STRING, SYMBOL_T, typeEquals, typeKey, UNDEFINED_T, VOID } from "../ir/nodes.js"; | ||
|
|
||
| import { isJsSourceFile, isNodeTypesPath } from "./program.js"; | ||
| import { isJsSourceFile, isMidiTypesPath, isNodeTypesPath } from "./program.js"; |
Contributor
There was a problem hiding this comment.
Accidental midi type-mapping additions in types.ts reference undefined symbols (isMidiTypesPath, IR kinds midiInput/midiOutput), breaking the compiler build.
| finalizeRecursive(t: ts.Type, fields: { name: string; type: IrType }[], indexValue?: IrType, declaredOrder?: string[]): string { | ||
| const id = this.recIds.get(t); | ||
| if (id === undefined) throw new InternalCompilerError("shape registry bug: finalizeRecursive without a placeholder"); | ||
| if (id === undefined) throw new Error("shape registry bug: finalizeRecursive without a placeholder"); |
Contributor
There was a problem hiding this comment.
Internal compiler invariant failures were downgraded from InternalCompilerError to generic Error, so callers can no longer distinguish compiler bugs from environmental failures.
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.
Problem
In
--dynamicmode, several type mapping gaps caused SC errors or prevented npm-origin values from flowing as island handles:TConfig & { schemaType: "widget" }whereTConfig extends z.ZodTypeAnyfailed with SC2008 because the TypeParameter had no static mapping and no constraint fallback.(): JSX.Elementfailed becauseReactElementhasany-typed fields, making the return type unmappable.--dynamicmode, poisoning intersection/record shapes they appeared in.unknownin catch/error handlers —dynFallbackTypedidn't mapunknowntodynin--dynamicmode, blockinginstanceof Error/typeofnarrowing.jsvalLiftablerequiredcanMarshalTypedFuncIntoIslandeven in--dynamicmode where all functions can be wrapped as host callbacks.Fix
types.ts: TypeParameter constraint fallback; function param/return JSVAL fallback; multi-sig→JSVAL with stdlib carve-out (node:child_process.spawnSync etc. have static lowerings and must not map to JSVAL)checker.ts: exposegetBaseConstraintOfTypeonCheckerFacadelowerer.ts:unknown→dynindynFallbackType;funcshort-circuit injsvalLiftablefor--dynamicVerification
Eliminated hundreds of spurious SC2008 and SC1090 errors in a ~600-file program.
packages/compilerbuilds clean.