Unify name mangling rules

[tsnobip]

Since BuckleScript is going to release a v8 and maybe Reason a v4, I think now could be a good time to fix name mangling issues once for all and stick to one rule.

Right now we have this Reason code:

let foo = {"_to": 10, "_type": 1};
type bar = {
  _to: int,
  _type: string,
};
let bar = {_to: 10, _type: "bar"};
ReactDOMRe.renderToElementWithId(<input type_="text" />, "preview");

compiling to:

var foo = {
  to: 10,
  type: 1
};
var bar = {
  _to: 10,
  _type: "bar"
};
ReactDOMRe.renderToElementWithId(React.createElement("input", {
          type: "text"
        }), "preview");

which is definitely not coherent.

I'd be in favor of using trailing underscores since leading underscores is already used to name unused variables but I'd be happy with any coherent rule.

[jordwalke]

Thanks for bringing this up. Can you describe your goals for unifying the name mangling rules? You want a consistent way across what specifically?

[tsnobip]

My goal would be to have the same name mangling rule to remember for Bucklescript, Reason and JSX instead of at least 3 rules right now.

Something like: if you want to have in JS a variable or object field named after a word reserved in BuckleScript, Ocaml or Reason, add a trailing underscore to the reserved name and it will be output correctly in JS, whether you use a record, a JS object or a React component.

I'm talking as a user, I'm sure there are tons of things to consider before changing this behavior since I know things went wrong at least once with method mangling and I'm not even sure in which of the 3 repos (BS, Reason, RR) things have to change but I think it will improve devs' quality of life quite a lot when writing bindings.

[jordwalke]

These are really easy to fix or improve - the only challenge is that last time we fixed them to be consistent everywhere, it ended up breaking existing assumptions that relied on the inconsistent behavior. So let's say you have a perfect approach "x" in mind. Changing the parser from imperfect approach to x, will change the compiled output of other libraries that use Reason under the assumption of the imperfect approach.

So the way forward is that we will make a breaking change in the parser, but it fixes it from a completeness/consistency perspective, while: Allowing each project that uses Reason to opt into its own version of the parser - allowing two versions to simultaneously coexist. There may be some surprises when someone opts into the new/improved form, but calls an existing library written under the old (imperfect) assumptions. But over time everything should converge to consistency and completeness. What is unacceptable (we've discovered) is to make any kind of changes (even if improvements) in the Reason parser name mangling, that changes the parsing of existing libraries who did not opt into the new improvements/version.

There may be some assumptions that BuckleScript makes about how the mangling should work - but BuckleScript's vendored Reason should make the changes necessary to that vendored Reason parser in sync with its own assumptions.

Thank you for bringing it up in an issue so we can track it.

[tsnobip]

Thank you Jordan for considering this issue seriously, I think in the long term these apparent "small fixes" can really smooth out the developer experience.

Of course I realize it can break a lot of things.

Maybe we could keep the old behavior of trimming leading underscores with depreciation warnings and add the new behavior while documenting it as the recommended way since the two rules are not mutually exclusive. It could help reduce the risks.

[TheSpyder]

What is unacceptable (we've discovered) is to make any kind of changes (even if improvements) in the Reason parser name mangling, that changes the parsing of existing libraries who did not opt into the new improvements/version.

Would it be more acceptable on a major version boundary? This might be a good change to target for v4 🤔

[jordwalke]

Would it be more acceptable on a major version boundary? This might be a good change to target for v4 🤔

No actually it's not even acceptable to change the meaning of code that was authored in v3, just because some other package uses a v4 which is why this is difficult. If someone writes something in v3, and it is being called from another package written in the next version, we cannot change what the called code written in v3 compiles to.

[TheSpyder]

Doesn't that mean there can never be a breaking change? We survived big upheaval in the move from v2 to v3, although the project is a lot more popular now, but it would be a shame if we were never able to make changes like that again.