Skip to content

Conversation

@cknitt
Copy link
Member

@cknitt cknitt commented Nov 16, 2025

Fixes #7954

@cknitt cknitt changed the title Fix printing of optional record fields in error messages Fix printing of optional record fields in pattern matching errors Nov 16, 2025
@cknitt cknitt marked this pull request as draft November 16, 2025 19:46
@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 16, 2025

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@8019

@rescript/darwin-arm64

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

@rescript/darwin-x64

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

@rescript/linux-arm64

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

@rescript/linux-x64

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

@rescript/runtime

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/runtime@8019

@rescript/win32-x64

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

commit: 994427f

@cknitt cknitt force-pushed the 7954 branch 4 times, most recently from d0c6eb2 to 5eb2fab Compare November 17, 2025 08:49
@cknitt cknitt marked this pull request as ready for review November 17, 2025 09:19
@cknitt cknitt requested review from cristianoc and zth November 17, 2025 09:19
Copy link
Collaborator

@cristianoc cristianoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, though an optional field of option type is very close to a time bomb.
I would be nearly tempted to issue a warning any time one uses it (that can be silenced in the 0.1% of cases where you actually need it).
The fact that the semantics is NOT what js does, makes it immediately confusing to people anyway.

@cknitt
Copy link
Member Author

cknitt commented Nov 18, 2025

@cristianoc This is not only for optional fields with options.

E.g.,

type t = {b?: result<string, string>}

let f = v =>
  switch v {
  | {b: Ok(x)} => x
  | {b: Error(y)} => y
  }

currently gives the error

You forgot to handle a possible case here, for example: 
| {b: None, _}

@cknitt cknitt merged commit 8988351 into rescript-lang:master Nov 18, 2025
35 of 37 checks passed
@cknitt cknitt deleted the 7954 branch November 18, 2025 15:56
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.

[bug] switching on optional field with type option results in totally weird warning

2 participants