Skip to content

make mutation of private record mutable fields a configurable warning instead of a hard error#8366

Open
tsnobip wants to merge 3 commits into
masterfrom
allow_mutation_private_record
Open

make mutation of private record mutable fields a configurable warning instead of a hard error#8366
tsnobip wants to merge 3 commits into
masterfrom
allow_mutation_private_record

Conversation

@tsnobip

@tsnobip tsnobip commented Apr 19, 2026

Copy link
Copy Markdown
Member

this PR allows this:

@@warning("-111")

module Foo: {
  type t = private {
    mutable foo: int,
  }
  let make: int => t
} = {
  type t = {
    mutable foo: int,
  }
  let make = foo => {foo: foo}
}

let foo = Foo.make(10)

foo.foo = 5

Using private for records is very convenient to bind to JS classes so you can easily access properties and methods while preventing object creation. Though, before this PR, the mutation of fields marked as mutable inside private records was not possible and that is a common limitation inside WebAPI bindings, for DOM.node for example where textContent should be mutable.

@tsnobip tsnobip force-pushed the allow_mutation_private_record branch from 3a8f357 to 2bcf3f7 Compare April 19, 2026 09:43
@pkg-pr-new

pkg-pr-new Bot commented Apr 19, 2026

Copy link
Copy Markdown

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@8366

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@8366

@rescript/darwin-x64

npm i https://pkg.pr.new/@rescript/darwin-x64@8366

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@8366

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@8366

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@8366

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@8366

commit: 25e0fc3

@tsnobip tsnobip force-pushed the allow_mutation_private_record branch from f33af2d to 12f8390 Compare April 19, 2026 12:28
@tsnobip tsnobip force-pushed the allow_mutation_private_record branch from 12f8390 to 57f0c88 Compare April 19, 2026 12:40
@tsnobip tsnobip force-pushed the allow_mutation_private_record branch from fea52d9 to 129786d Compare June 10, 2026 14:56
@tsnobip tsnobip changed the title allow mutation of private record fields with @allowMutation make mutation of private record mutable fields a configurable warning instead of a hard error Jun 10, 2026
@tsnobip tsnobip force-pushed the allow_mutation_private_record branch from 129786d to 39fbb48 Compare June 10, 2026 14:59
@tsnobip

tsnobip commented Jun 10, 2026

Copy link
Copy Markdown
Member Author

thanks to @fhammerschmidt's idea, I simplified the design and the implementation by making this a configurable warning, let me know what you think @zth.

tsnobip added 2 commits June 10, 2026 17:06
should be reversed at some point to become the
default behavior you could opt-out with `@DisallowMutation`
@tsnobip tsnobip force-pushed the allow_mutation_private_record branch from 39fbb48 to 517edf9 Compare June 10, 2026 15:15
@tsnobip tsnobip requested a review from zth June 10, 2026 15:16
Comment thread compiler/ml/typedecl.ml
@github-actions

Copy link
Copy Markdown

@tsnobip tsnobip force-pushed the allow_mutation_private_record branch from 517edf9 to 25e0fc3 Compare June 10, 2026 16:03
@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.95%. Comparing base (3f48a4d) to head (25e0fc3).

Files with missing lines Patch % Lines
compiler/ml/typecore.ml 77.77% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8366   +/-   ##
=======================================
  Coverage   60.95%   60.95%           
=======================================
  Files         374      374           
  Lines       54117    54124    +7     
=======================================
+ Hits        32986    32992    +6     
- Misses      21131    21132    +1     
Files with missing lines Coverage Δ
compiler/ext/warnings.ml 71.67% <100.00%> (+0.24%) ⬆️
compiler/ml/typecore.ml 86.11% <77.77%> (-0.02%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

4 participants