Repository navigation
fix(js/ts/python): pass explicit ctor args to primary base - #5247
MangelMaxime wants to merge 16 commits into
Conversation
Python Type Checking Results (Pyright)
Excluded files with errors (4 files)These files have known type errors and are excluded from CI. Remove from
|
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved constructor-generation defects can produce invalid parameter lists, corrupt forwarded arguments, and break initialization order, causing parse errors and incorrect runtime behavior.
7 open issues
Reserve generated base parameter names to prevent collisions · New Sanitize base parameter names for the target language · New Include base parameters when matching constructor argument types · New Allocate a unique name for the ParamArray argument · New Preserve named unit arguments in base constructor calls · New Run child field initializers only after the base constructor · New Convert ParamArray before evaluating base-call arguments · New
What changed in this PR
Addresses #5227 by forwarding explicit constructor arguments to primary-constructor bases in Fable’s JavaScript, TypeScript, and Python targets, preserving inheritance and base initialization.
Changes:
- Adds primary-base argument forwarding and rejects unsupported multi-level inheritance chains.
- Updates ParamArray forwarding in JS/TS and Python constructor wrappers.
- Adds regression tests for inherited members, type tests, generics, naming collisions, optional arguments, and ParamArray arguments.
| File | Description |
|---|---|
tests/Python/TestType.fs |
Adds Python constructor-inheritance regression tests. |
tests/Js/Main/TypeTests.fs |
Adds shared JS/TS constructor-inheritance regression tests. |
src/Fable.Transforms/Python/Fable2Python.Transforms.fs |
Converts constructor ParamArrays and forwards variadic arguments. |
src/Fable.Transforms/FSharp2Fable.Util.fs |
Builds primary-base arguments and reports unsupported inheritance chains. |
src/Fable.Transforms/FSharp2Fable.fs |
Forwards base arguments during explicit constructor lowering. |
src/Fable.Transforms/Babel/Fable2Babel.fs |
Spreads ParamArray arguments in exposed constructor wrappers. |
Review effort: Balanced
8f8777b to
afe1e98
Compare
af42573 to
49f0a3d
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Constructor lowering replaces explicit base initialization with defaults, corrupting inherited state, and can evaluate throwing field reads before the base constructor, altering its effects or exceptions.
2 open issues
7 resolved since last review
Allocate a unique name for the ParamArray argument Include base parameters when matching constructor argument types Sanitize base parameter names for the target language Reserve generated base parameter names to prevent collisions Convert ParamArray before evaluating base-call arguments Run child field initializers only after the base constructor Preserve named unit arguments in base constructor calls
Review effort: Balanced
| match param.Type with | ||
| | Fable.Option(genArg, isStruct) -> | ||
| Fable.Value(Fable.NewOption(None, genArg, isStruct), None) | ||
| | t -> Replacements.Api.defaultof com ctx None t |
| if | ||
| ownArgs |> List.exists (canHaveSideEffects com) | ||
| || bindings |> List.exists (snd >> canHaveSideEffects com) | ||
| then |


Fixes #5227.
On JS, TS and Python,
thenblocks andnew() as xnow also work in an explicit constructor whose base class has a primary constructor.