Fix null check in pattern matching for JSON.Array and JSON.Object by jagguji · Pull Request #8252 · rescript-lang/rescript

jagguji · 2026-02-23T14:40:33Z

When pattern matching on JSON values with both Array and Object cases without an explicit Null case, the generated JavaScript incorrectly omitted the null check before Array.isArray(), causing runtime failures when the value is null.

Root cause: typeof null === "object" in JavaScript, so the previous Object type check would incorrectly match null.

Changes:

In is_not_block_case: Always add null check for ObjectType
In add_runtime_type_check: Always exclude null from Object case

Fixes runtime error where null would incorrectly match Object case instead of falling through to the default/catch-all case.

#8251

When pattern matching on JSON values with both Array and Object cases without an explicit Null case, the generated JavaScript incorrectly omitted the null check before Array.isArray(), causing runtime failures when the value is null. Root cause: typeof null === "object" in JavaScript, so the previous Object type check would incorrectly match null. Changes: 1. In is_not_block_case: Always add null check for ObjectType 2. In add_runtime_type_check: Always exclude null from Object case Fixes runtime error where null would incorrectly match Object case instead of falling through to the default/catch-all case. Signed-Off-By: Claude <noreply@anthropic.com>

nojaf · 2026-02-24T08:23:12Z

Hello, thank you for your contribution!
Please run make format so CI can run.
And consider adding a build test for this.

jagguji · 2026-02-24T11:09:03Z

sure @nojaf will do it

jagguji · 2026-02-24T11:59:11Z

can u check now @nojaf

nojaf · 2026-02-24T12:09:04Z

Hi @cknitt, I'm less familiar with the build tests, does this look good to you?

jagguji · 2026-02-24T12:28:18Z

Hi @cknitt @nojaf can we pls look on it as a priority ?
As we have moved to this rescript v12 last week and because of this issue our production is getting delayed
Thanks

nojaf · 2026-02-24T13:12:51Z

Please run make format again and address the Biome lint suggestion.

Once CI reaches the CI / pkg-pr-new stage, it will publish an npm package to unblock you.

jagguji · 2026-02-24T13:47:27Z

hi @nojaf pkg-pr-new is skipped

nojaf · 2026-02-24T13:53:07Z

Well yes, because CI keeps failing:

❌ error in json_null_check with stderr:
Test failed with output:

node:internal/modules/cjs/loader:1386
  throw err;
  ^

Error: Cannot find module '/Users/runner/work/rescript/rescript/tests/build_tests/json_null_check/lib/Main.mjs'
    at Function._resolveFilename (node:internal/modules/cjs/loader:1383:15)
    at defaultResolveImpl (node:internal/modules/cjs/loader:1025:19)
    at resolveForCJSWithHooks (node:internal/modules/cjs/loader:1030:22)
    at Function._load (node:internal/modules/cjs/loader:1192:37)
    at TracingChannel.traceSync (node:diagnostics_channel:328:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:237:24)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:171:5)
    at node:internal/main/run_main_module:36:49 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

Node.js v22.22.0

Did you ran this locally? make test

jagguji · 2026-02-24T15:04:27Z

intiailly i was not able to run it but now it's i ran it and it works fine, can u check now ? and approve this workflow ?
@nojaf

pkg-pr-new · 2026-02-24T15:37:46Z

Open in StackBlitz

rescript

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

@rescript/darwin-arm64

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

@rescript/darwin-x64

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

@rescript/linux-arm64

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

@rescript/linux-x64

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

@rescript/runtime

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

@rescript/win32-x64

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

commit: 614c29a

nojaf · 2026-02-24T15:42:47Z

@jagguji as mentioned above:

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

Please try that out.

cristianoc · 2026-02-24T16:13:52Z

tests/tests/src/UntaggedVariants.mjs


 function classify$8(v) {
-  if (typeof v === "object" && !Array.isArray(v)) {
+  if (typeof v === "object" && v !== null && !Array.isArray(v)) {


This is a regression: null check not needed.
Btw the other test should also be of this form: a simple function in tests/tests/src so we can just look at the generated code, instead of an opaque case analysis with half a dozen cases

@cristianoc The null check IS needed because of JavaScript's typeof null === "object".

jagguji · 2026-02-24T16:53:47Z

@jagguji as mentioned above:
npm i https://pkg.pr.new/rescript-lang/rescript@8252
Please try that out.

hey i was trying it with yarn we were using the yarn 3.2.1, and it's not working
getting this YN0000: ┌ Resolution step
➤ YN0001: │ Error: rescript@https://pkg.pr.new/rescript-lang/rescript@8252 isn't supported by any available resolver
at Bd.getResolverByDescriptor (/Users/roshan.chourasiya/.cache/node/corepack/v1/yarn/3.2.1/yarn.js:396:1664)
at Bd.bindDescriptor (/Users/roshan.chourasiya/.cache/node/corepack/v1/yarn/3.2.1/yarn.js:396:1053)
at ee (/Users/roshan.chourasiya/.cache/node/corepack/v1/yarn/3.2.1/yarn.js:442:6954)
at async Promise.allSettled (index 0)
at async uo (/Users/roshan.chourasiya/.cache/node/corepack/v1/yarn/3.2.1/yarn.js:395:10390)
at async /Users/roshan.chourasiya/.cache/node/corepack/v1/yarn/3.2.1/yarn.js:442:8292
at async Je.startProgressPromise (/Users/roshan.chourasiya/.cache/node/corepack/v1/yarn/3.2.1/yarn.js:395:47994)
at async ze.resolveEverything (/Users/roshan.chourasiya/.cache/node/corepack/v1/yarn/3.2.1/yarn.js:442:6285)
at async /Users/roshan.chourasiya/.cache/node/corepack/v1/yarn/3.2.1/yarn.js:445:2132
at async Je.startSectionPromise (/Users/roshan.chourasiya/.cache/node/corepack/v1/yarn/3.2.1/yarn.js:414:3303)
➤ YN0000: └ Completed
➤ YN0000: Failed with errors in 0s 67ms

jagguji · 2026-02-24T17:36:35Z

hi @nojaf @cristianoc can u pls have a look at it again, and let me know if anything still open, i don't think u are understanding our situation our prod migration is delayed because of this can u pls fastrack this
Thanks !!

jagguji added 3 commits February 24, 2026 17:02

Apply formatting fixes

418ed5e

Add build test for JSON null check fix

b5ab009

Add comprehensive test cases for JSON null check fix

9b55cc6

Fix build test structure - add input.js and move to src/

d8256d0

Remove unused import to fix Biome lint

2f2313c

jagguji added 2 commits February 24, 2026 20:04

Remove unused variables and fix build test

bd4c20c

Format ReScript test file

614c29a

fhammerschmidt requested a review from cristianoc February 24, 2026 16:03

cristianoc reviewed Feb 24, 2026

View reviewed changes

Comments

Conversation

jagguji commented Feb 23, 2026

Uh oh!

nojaf commented Feb 24, 2026

Uh oh!

jagguji commented Feb 24, 2026

Uh oh!

jagguji commented Feb 24, 2026

Uh oh!

nojaf commented Feb 24, 2026

Uh oh!

jagguji commented Feb 24, 2026

Uh oh!

nojaf commented Feb 24, 2026

Uh oh!

jagguji commented Feb 24, 2026

Uh oh!

nojaf commented Feb 24, 2026

Uh oh!

jagguji commented Feb 24, 2026

Uh oh!

pkg-pr-new bot commented Feb 24, 2026

Uh oh!

nojaf commented Feb 24, 2026

Uh oh!

cristianoc Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

jagguji Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

jagguji commented Feb 24, 2026

Uh oh!

jagguji commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants