[JS]: Add support for node ESModules with --experimental-modules flag

[trxcllnt]

This PR adds support for using flatbuffers with node's new --experimental-modules flag which natively supports ESModules:

1. remove the this.flatbuffers = flatbuffers; line from the end of the source file
2. rename js/flatbuffers.js to js/flatbuffers.src.js, in order to maintain backwards-compatability
3. add an npm prepublishOnly script that creates two new (git-ignored) files:
   a. js/flatbuffers.js with this.flatbuffers = flatbuffers; appended to the end for CommonJS/RequireJS backwards-compat
   b. js/flatbuffers.mjs with export { flatbuffers }; appended to the end for ESModules
4. Removes the extension from the "main": "js/flatbuffers.js" entry in package.json (so it is now just "main": "js/flatbuffers")
5. Adds a second explicit "module": "js/flatbuffers.mjs" definition in the package.json, for compatibility with Closure Compiler
6. Updates the "files" entry in package.json to publish ["js/flatbuffers.js", "js/flatbuffers.mjs"]

Items 2-4 are necessary because node will automatically select either the "js" or "mjs" file depending on whether it was run with the --experimental-modules flag, but only if the "main" entry doesn't explicitly specify a file extension.

If you attempt to import a CommonJS module as an ESModule (either by explicitly importing the "js" version, or if there's no explicit extension and no "mjs" file available), node will synthesize a default export from the "module.exports" object, leading to the following error condition:

// $ node index.js
// works like a charm
const { flatbuffers } = require('flatbuffers');

// $ node --experimental-modules index.mjs
// throws "SyntaxError: The requested module does not provide an export named 'flatbuffers'"
import { flatbuffers } from 'flatbuffers';

Item 5 is necessary support newer builds of Closure Compiler that support compiling ESModules with the module_resolution: "NODE" compiler flag:

closure-compiler --module_resolution=NODE \
  # select dep package.json "module" entries before "main"
  --package_json_entry_names=module,main \
  --js=node_modules/flatbuffers/package.json \
  --js=node_modules/flatbuffers/js/flatbuffers.mjs \
  --entry_point=index.js

For context: the Apache Arrow project is based on flatbuffers, and we compile ES6 -> ES5 JS bundles with closure-compiler + advanced optimizations.

Recently closure-compiler's ESModules support has improved to the point where we can use it directly, instead of duplicating and maintaining dependency sources in our project. We also support importing Arrow as ESModules in node directly with the --experimental-modules flag.

The last step in this process is helping ensure our dependencies expose closure-compiler and node v8.6.0+ compatible ESModules. Please let me know if there's anything else I can do to help get this PR accepted, and a new version published soon. Thanks!

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
• In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

[trxcllnt]

I signed the CLA with the email [email protected].

[googlebot]

CLAs look good, thanks!

[aardappel]

Not being a JS expect, it is not obvious to me that this supports all JS versions on all platforms, in/outside browser, TypeScript, Windows etc. This makes use of some scripting to generate flatbuffers.js which makes me uneasy, can you explain how this supports all possible platforms?

@krojew for TS. Any other JS users want to chime in?

[trxcllnt]

@aardappel This makes use of some scripting to generate flatbuffers.js which makes me uneasy, can you explain how this supports all possible platforms?

Absolutely! You can preview changes to the npm package proposed in this PR at trxcllnt/flatbuffers-esm. The Arrow project is presently depending on this instead of the mainline flatbuffers npm package.

Excepting those pulling the file from source-control, this PR won't affect anyone currently relying on the existing flatbuffers npm module. If it's believed that people are pulling js/flatbuffers.js straight from source-control, we can remove the entry for js/flatbuffers.js in the .gitignore.

This PR adds support for importing flatbuffers in newer versions of node with native ESModules support enabled. Without this change, compilers (like TS) will happily compile TypeScript source to ES6+ with ESModule imports, but users will see the above exception trying to execute the code in node with the --experimental-modules flag. Additionally, the new mjs file is compatible with newer versions of Google Closure Compiler without modification.

I'm not necessarily a fan of the scripts that create the flatbuffers.js and flatbuffers.mjs CommonJS and ESModule files, but the approach avoids both committing duplicate files to source control, and adding additional dependencies for build tooling.

The JS lib seems mature enough that it doesn't require frequent updates or complex build tools. It's reasonable to revisit this assumption if the project does one day resume extensive active development.

Building on Windows:

It's my understanding the cp command in the append-cjs-export and append-esm-export scripts aren't compatible with Windows. If you don't mind adding a dev-dependency, it's common to import and use the shx module in npm scripts instead (shx cp instead of cp).

A bit about me:

I'm @ReactiveX core team member and Apache contributor on Arrow. I coauthored rxjs (which ships in Angular2+ core) and am responsible for build tooling and compatibility for a number of other projects.

A bit about Arrow:

The Arrow project is written in TypeScript. We compile the TS source to a 3x3 matrix of supported JS versions and module formats, as well as ES6 + ESModule ".mjs" files for use in node with the --experimental-modules flag. We compile with the most restrictive TypeScript compilation settings available, and use closure compiler, webpack, and uglifyjs-es at different points in the build.

At the moment we have a battery of 117,420 unit and integration tests that are executed against each of compilation targets to ensure compatibility and correctness across the entire matrix.

[krojew]

From the TS point of view, I don't see any problems. TS code only assumes flatbuffers are available (via typings) and the resulting JS code will have it available using the user-chosen import method, whatever that will be.

[aardappel]

I'd prefer it if this keeps working for people who work directly with our repo, I am sure there are some :)

If the cp in package.json only needs to work for the person uploading to npm (me), then I guess it is ok if it doesn't work on Windows I guess, but better yet if all this copying wasn't required at all.

We've tried to make changes to importing before, and it broke people using the Closure compiler. Is this likely to affect it? (again, not a JS expert :)

[trxcllnt]

@aardappel I don't expect it to break anybody using Closure compiler. The full diff of changes to the flatbuffers.js file that's downloaded from npm is:

- // Exports for Node.js and RequireJS
- this.flatbuffers = flatbuffers;
/// @endcond
/// @}
+ 
+ // Exports for Node.js and RequireJS
+ this.flatbuffers = flatbuffers;

To any JS interpreter, we just moved the /// @endcond comments up a few lines. As best as I can tell from the Closure compiler docs + testing with the online API, CC doesn't care whether the export is inside or outside the @cond annotation comment brackets.

I can't say conclusively whether this would matter to older versions of the Closure compiler, but since I can't find any references to @cond in the docs, and a google search for closure compiler "@cond" yields no results, I think we're safe.

We compile ES6 to ES5 in the Arrow build via Closure compiler with advanced optimizations. This is a relatively new feature in Closure compiler, and they don't allow you to mix JS module systems, so adding the .mjs file (with the ES6-style export) will actually enable people to use newer versions of Closure compiler going forward.

[trxcllnt]

@aardappel I'd prefer it if this keeps working for people who work directly with our repo, I am sure there are some :)

Ah, then feel free to commit the flatbuffers.js and flatbuffers.mjs files to git as well :-)

[aardappel]

I'm confused, are you going to make changes such that flatbuffers.js is not removed/ignored?

[trxcllnt]

@aardappel oh, yeah! sorry, I wasn't sure if you wanted to defer that decision until after this PR is merged or not. Will commit that here in a bit.

[trxcllnt]

@aardappel added the files and rebased from master 👍

[aardappel]

Ok, so now I wonder, given that flatbuffers.js is the source of truth for future modifications, how are those changes supposed to end up in the other 2 copies? Aren't we better off if the script copies from flatbuffers.js instead? That way we don't even need to have the copies in the repo?

[trxcllnt]

@aardappel after this PR, flatbuffers.src.js is the source of truth, and flatbuffers.js and flatbuffers.mjs are the derived files.

• npm run append-cjs-export creates the flatbuffers.js file, with the legacy export appended
• npm run append-esm-export creates the flatbuffers.mjs file, with the ES6 export appended

Node needs the .js and .mjs files to share the same name (with different extensions) to work, and the .mjs file can't have both the legacy export and ES6 export because that breaks Closure compiler.

That way we don't even need to have the copies in the repo?

Yes that would be my preference (and why I'd gitignore'd the generated files initially), but you raised concerns people may be linking to the files directly via git instead of using the published packages, and I'm happy to defer to you here.

[aardappel]

Well, I'd prefer it if flatbuffers.js is the source of truth, and the other 2 are copied as needed by the script, since that fits what we've been doing so far. Having 3 identical files is very confusing for contributors. The copied files don't need to be in the repo, I think.

[trxcllnt]

@aardappel we can do that, but removing the legacy commonJS export from flatbuffers.js will break backwards compatibility for builds that have the path to js/flabuffers.js hard-coded.

If you're fine with that, then we'd need to rename the derived files to something like flatbuffers.lib.js and flatbuffers.lib.mjs, and set the "main" field in the package.json to js/flatbuffers.lib (without the file extension).

Do you want to schedule a quick hangout or slack call to discuss options? You can reach me at the email in my github profile, or via [email protected]

[trxcllnt]

@aardappel alternatively I could use awk to find and replace the legacy commonJS export with an ES6 export to synthesize the mjs file on publish, but I figured a PR with an awk script might be even less well-received ;-j

[aardappel]

@trxcllnt I've sent you a hangouts invite.

Apologies if this is drawn-out, my understanding of the JS eco-system is minimal, and I am wary of breaking existing users (and copies of in repos :)

Is there no way to if-then around the offending statement?
If awk is not guaranteed to be installed, is there another tool? sed? We already concluded this wasn't going to run on Windows I think.

[trxcllnt]

@aardappel awesome, thanks! I'm available now in hangouts

[trxcllnt]

@aardappel alright, good to go! I'd mixed up sed and awk, meant to say sed before. So now the js/flatbuffers.js source file remains unchanged/the source of truth, and npm publish synthesizes the js/flatbuffers.mjs file like this:

$ sed "s/this.flatbuffers = flatbuffers;/export { flatbuffers };/" js/flatbuffers.js >> js/flatbuffers.mjs

[aardappel]

LGTM!
I previously just used npm publish upon new version releases, is that any different now?

[trxcllnt]

@aardappel shouldn't be. If you're using npm >= 4.0.0, npm will execute the "prepublishOnly" script just before pushing the files. If you're using a version of npm < 4.0.0, you can do npm run append-esm-export to create the mjs file first, then do the normal npm publish.

[aardappel]

I'm on 3.10.10 for some reason. Noted :)

Thanks for your persistence :)