fix(compiler): Generate backwards-compatible code to not crash on old browsers

[Jimbly]

Since adopting @messageformat/core we've been running into various issues for some users on old browsers. For most of the issues the combination of Babel and polyfills handles it, but it appears this specific code is generating a string that contains an arrow function that's being passed to eval(), which of course Babel can't deal with. Trivial change, tested on Chrome 38 on our game (https://worlds.frvr.com, still have the old version live), mostly just had to change a bunch of test case outputs to match.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: Jimbly / name: Jimb Esser (aabcbb1)

[eemeli]

Do I understand right that you're compiling ICU MessageFormat strings in your client runtime, where this line is then throwing an error in really old browsers?

messageformat/packages/core/src/messageformat.ts

Line 264 in bde8927

const fn = new Function(...nfArgs, fnBody);

Have you considered using compileModule() to instead do the parsing during your application build, and avoid needing to call new Function() during your runtime?

[Jimbly]

Do I understand right that you're compiling ICU MessageFormat strings in your client runtime, where this line is then throwing an error in really old browsers?

I didn't do the implementation (just trying to fix things that broke afterwards ^_^), but yeah, we are. Seem to mostly be getting errors in the wild from "Samsung Internet" and old versions of Chrome. We do allow our volunteer translators to test their changes in-game, so do need the compiling to happen in the runtime (without access to source code / build pipeline), although in theory that could be in both the build pipeline and the runtime and the runtime stuff for translators could be limited to only new browsers.

Have you considered using compileModule() to instead do the parsing during your application build, and avoid needing to call new Function() during your runtime?

I didn't know that was an option, but will pass the suggestion on to the engineer responsible, but that would likely be a more involved change.

[eemeli]

Have you considered using compileModule() to instead do the parsing during your application build, and avoid needing to call new Function() during your runtime?

I didn't know that was an option, but will pass the suggestion on to the engineer responsible, but that would likely be a more involved change.

That's really the main recommended usage pattern for this library, tbh. Doing so would save you from needing to deploy the compiler at all in your client runtime. But yeah, I see how that would be a more involved change esp. if you need to figure out a different way for testing translations.

Now, rather than effectively rolling back #309 and dropping the fat-arrow notation, I'd much prefer making this configurable. In other words, defaulting to the terser notation but allowing for a new option that would use function(d) { return ... } instead of d => .... Would you be intersted in updating this PR in that direction?

Btw, the Node.js 19 test failures aren't your fault; those seem to be caused by an ICU 72 change in the whitespace used before the AM/PM marker.

[Jimbly]

Would you be intersted in updating this PR in that direction?

I'm not exactly sure where to start with that, but if you can point at some other code taking options I could take a poke at it. Alternatively, do we know if we're generating code at run-time (to pass to Function) or at build-time, and could always use the function syntax at run-time?

That being said, I just realized it actually would be pretty easy to just replace window.Function with a simple polyfill (just for MessageFormat) before our call to compile() and that would perhaps work around this in a less intrusive fashion.

[eemeli]

@Jimbly:
I'm not exactly sure where to start with that, but if you can point at some other code taking options I could take a poke at it.

Probably something like the requireAllArguments option added in #267 is pretty close to what this would look like, as it similarly affects the compiler behaviour.

Alternatively, do we know if we're generating code at run-time (to pass to Function) or at build-time, and could always use the function syntax at run-time?

If you're doing it at build time, you should either find calls to compileModule() somewhere in your build code, or that you're using the CLI tool, Webpack loader or Rollup plugin. See here for more info: http://messageformat.github.io/messageformat/use/

That being said, I just realized it actually would be pretty easy to just replace window.Function with a simple polyfill (just for MessageFormat) before our call to compile() and that would perhaps work around this in a less intrusive fashion.

Tbh, that sounds like a more intrusive option to me. But sure, it might work.

[eemeli]

Closing this as presumably solved by a user-space change.