Skip to content

[JS/TS/Python] Fix record/value type constructor including static backing fields as parameters#4405

Merged
MangelMaxime merged 5 commits intomainfrom
fix-invalid-constructor
Mar 17, 2026
Merged

[JS/TS/Python] Fix record/value type constructor including static backing fields as parameters#4405
MangelMaxime merged 5 commits intomainfrom
fix-invalid-constructor

Conversation

@MangelMaxime
Copy link
Copy Markdown
Member

@MangelMaxime MangelMaxime commented Mar 14, 2026

Summary

When a record or value type is augmented with static let or static member val, the F# compiler generates static backing fields that appeared in ent.FSharpFields. These were incorrectly included as constructor parameters, causing two distinct bugs:

  1. static let fields shift the index-based args[i] assignments in generated constructors, so {id = 1} could return id = 0 instead.
  2. static member val backing fields (e.g. empty@ from static member val empty = ...) were treated as instance fields, causing all instances to share the same mutable backing store.

Target changed

This PR is touching JavaScript, Python targets.

Other targets are left untouched because it was generating invalid code and I am no expert in those targets.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 14, 2026

Python Type Checking Results (Pyright)

Metric Value
Total errors 18
Files with errors 4
Excluded files 4
New errors ✅ No
Excluded files with errors (4 files)

These files have known type errors and are excluded from CI. Remove from pyrightconfig.ci.json as errors are fixed.

File Errors Status
temp/tests/Python/test_applicative.py 12 Excluded
temp/tests/Python/test_hash_set.py 3 Excluded
temp/tests/Python/test_nested_and_recursive_pattern.py 2 Excluded
temp/tests/Python/fable_modules/thoth_json_python/encode.py 1 Excluded

@MangelMaxime
Copy link
Copy Markdown
Member Author

Something is not right in this PR because I definitely added tests and reverted the changes for Rust because it was breaking for that target.

@MangelMaxime MangelMaxime force-pushed the fix-invalid-constructor branch from 11e045a to 86197f8 Compare March 15, 2026 14:42
@MangelMaxime MangelMaxime requested review from dbrattli, ncave and nojaf and removed request for nojaf March 15, 2026 14:43
@MangelMaxime MangelMaxime force-pushed the fix-invalid-constructor branch from 86197f8 to b7b43ce Compare March 15, 2026 14:54
@MangelMaxime MangelMaxime changed the title Fix record/value type constructor incorrectly including static backing fields as parameters [JS/TS/Python] Fix record/value type constructor including static backing fields as parameters Mar 15, 2026
@fable-compiler fable-compiler deleted a comment from claude bot Mar 15, 2026
@MangelMaxime
Copy link
Copy Markdown
Member Author

@dbrattli It seems like the 4 new errors in Python type checking are caused by the line being too long.

image

How do we deal with that?

Those lines are not the only ones too long in that file. Does it means we can just merge it and those new errors will be ignored in future run?

@dbrattli
Copy link
Copy Markdown
Collaborator

@MangelMaxime That is strange. I though we ran ruff-formatting on the generated output. Let me have a look ...

@dbrattli
Copy link
Copy Markdown
Collaborator

@MangelMaxime Check if this is needed for JS. I fixed it for Python: Record reflection info includes static backing fields

When a record type has static let or static member val bindings, the backing fields for these statics are included in the reflection metadata alongside the actual instance fields. For example:

type RecordWithStaticLet =
{ id: int }
static let _a = 2
static member a = _a

The generated record_type(...) call reports fields [("_a", ...), ("init@...", ...), ("id", ...)] instead of just [("id", ...)]. This affects serialization, structural equality, and anything that uses reflection info at runtime.

Fix: In transformRecordReflectionInfo, filter out static fields from ent.FSharpFields before generating the field list. The Python fix filters with fi.IsStatic in a Seq.choose. The equivalent change in Fable2Babel.fs would be at line 178:

// Before
ent.FSharpFields
|> List.map (fun fi -> ...)

// After
ent.FSharpFields
|> List.choose (fun fi ->
if fi.IsStatic then None
else Some(...))

This is the same pattern already applied to constructors in commit b7b43ce.

MangelMaxime and others added 5 commits March 17, 2026 20:43
Filter static backing fields from record reflection metadata and generate
ClassVar annotations on dataclasses so Pyright recognizes static attributes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@MangelMaxime MangelMaxime force-pushed the fix-invalid-constructor branch from 50c9bdc to 20758ba Compare March 17, 2026 19:43
@MangelMaxime
Copy link
Copy Markdown
Member Author

Thank you I believe I indeed needed the same fix for JavaScript

@MangelMaxime MangelMaxime merged commit 321bbdb into main Mar 17, 2026
24 checks passed
@MangelMaxime MangelMaxime deleted the fix-invalid-constructor branch March 17, 2026 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants