Invalid function call in generated output

[alex35mil]

// External binding to global "test"
@val external _test: (string, (unit => unit) => unit) => unit = "test"

// Wrapper function shadows the external's target name
let test = (name, callback) => {
  _test(name, _done => callback())
}

// BUG: This should call global test(), but calls local wrapper instead
let broken = () => {
  _test("async", done => done())
}

Generates:

// Generated by ReScript, PLEASE EDIT WITH CARE


function test(name, callback) {
  globalThis.test(name, _done => callback());
}

function broken() {
  test("async", done => done());
}

export {
  test,
  broken,
}
/* No side effect */

[dsiu]

Adding@scope("globalThis") to external binding seems to fix this issue. I ain't sure if this is the righ behavior since it was introduced in v12.0.2.

rescript-jest is failing because of this issue and I had to submitted a similar pull request // @glensc

// External binding to global "test"
@val @scope("globalThis")
external _test: (string, (unit => unit) => unit) => unit = "test"

// Wrapper function shadows the external's target name
let test = (name, callback) => {
  _test(name, _done => callback())
}

// BUG: This should call global test(), but calls local wrapper instead
let broken = () => {
  _test("async", done => done())
}

// Generated by ReScript, PLEASE EDIT WITH CARE


function test(name, callback) {
  globalThis.test(name, _done => callback());
}

function broken() {
  globalThis.test("async", done => done());
}

export {
  test,
  broken,
}
/* No side effect */

[glennsl]

Here's the compiler output of the code in OP from both 12.0.1 and 12.0.2, for comparison:

12.0.1:

function test$1(name, callback) {
  test(name, _done => callback());
}

function broken() {
  test("async", done => done());
}

export {
  test$1 as test,
  broken,
}

12.0.2:

function test(name, callback) {
  globalThis.test(name, _done => callback());
}

function broken() {
  test("async", done => done());
}

export {
  test,
  broken,
}

I suspect this was caused by #8098, which seems to have two effects in this scenario:

1. It rewrites test to globalThis.test, in some cases, but not all. After reading both the PR description and code, it's still unclear to me both what the intended rules and actual rules for this are.
2. It prevents subsequent bindings from detecting the external name as a duplicate, and therefore prevents the name mangling that previously prevented this issue from occurring.

Fixing either of these should solve the issue.