Support .mjs and .bs.mjs for Node.js ES-module-support, and lower "suffix" option into "package-specs", #4116
Conversation
8eb0318 to
c07dcf0
Compare
|
If you prefer GitHub's "files" view, here's a version of this pull-request rebased on top of the whitespace-and-formatting fixes I'm making along the way: |
Disagree. Only for node you might want to have |
c07dcf0 to
e3d6174
Compare
|
@jfrolich can you elaborate? I fully admit that I don't know Everything, Ever; but to the best of my knowledge,
That said, I don't want to sidetrack this; and given that "not changing the default" is always a good idea for backwards-compatibility, even having one person have spoken out against doing so makes me tend to think “sure, let's leave it be, then.” To be clear, I'd prefer we future-proof the default while BuckleScript is still small; but it's far from my primary concern here. Feel free to weigh in, or not. ¯\_(ツ)_/¯ |
|
@ELLIOTTCABLE: Sorry for my short answer. |
|
@jfrolich 👍, sounds good to me. I prefer to be forward-looking, but not at the expense of practicality. |
1077cd2 to
177dc12
Compare
|
how would people really benefit es6 module? This is not Es6 compilant, so it still needs a bundler |
Plenty of ways, but I'd also posit that that's not our concern — instead, it's best to be as compliant as possible, and let people use us in whatever way works best for them!
Several things:
|
b4fd150 to
9f9353f
Compare
These are the portions of the codebase changed in later commits to this branch. I've rebased any whitespace / formatting-only changes back into this commit, to produce a slightly-less-noisy diff.
This gets everything compiling again, but we're far from done! 😫
Additionally involed: - Split `cmj_case` type into new, simplified `leading_case` type and aformentioned `package_info` changes - Store file-extension(s) in `.cmj` files - Bump `cmj_magic_number`, as it now contains new information Backwards-compatibility is maintained.
In addition, add `.bs.cjs` alongside `.bs.mjs` as a cleaning-supported file-extension.
3de2fb8 to
ce8e5c9
Compare
|
this is already implemented in master, thanks for the pioneering work |
Now that
.mjssupport has been made the default behaviour in Node.js, I think it's time to finally add first-class.mjsand ES-Module support to BuckleScript.My proposal is thus: that instead of the single, global boolean flag that decides between
.bs.jsand plain.js,... that we specify the file-extension per
-bs-package-output, i.e. lower"suffix"into the individual module-format-objects:Further, I propose the following changes to the default behaviour (which are, effectively, the logical conclusion of #2307):
"in-source"builds should default to the.bs.*suffix, out-of-source builds should not;"module": "commonjs"builds should default to*.js, and"es6"builds should default to*.mjs(With these defaults, the configuration "just works" with all existing JavaScript bundlers I'm aware of (Rollup, Webpack, etc) — i.e.
import ... from 'bs-library/src/path'will resolve tobs-library/src/path.jsin a CJS build, andbs-library/src/path.mjsin an ESM build.)Finally, in the process, as suggested by @JohnGurin, I'm removing the hard constraints on the value of
"suffix": it can be any string, but any value other than the four described above will print a warning-message. (In particular, the tree-cleaning involved with"in-source" && "suffix": ".bs.js"builds will only happen for, specifically,".bs.js"and".bs.mjs", not for".bs.mycoolmadeupextension".)This will almost certainly have to wait until a major-release, as it unfortunately looks like it involves a slight change to the
.cmjformat.Todo:
(subject to change, of course, as I spelunk the related source-code!)
bsb: Replace"suffix"-detection with detection of subfields of"package-specs",bsb: Add backwards-compatible detection and a deprecation notice for root-level"suffix"bsb: Update tree-cleaning operation to clean".bs.mjs"as well, for"in-source"buildsjscomp: Remove the-bs-suffixflag, and instead add a suffix-field to the-bs-package-outputfield (amdjs:lib/amdjs:.bs.mjs)bsb: Update Ninja-generation to use new-bs-package-outputform instead of-bs-suffixjscomp: Either removecmj_casefrom the.cmj-format entirely, make it non-dependant upon suffix, or separate it into two fields in the.cmj:caseandsuffixjscomp: (IN PROGRESS) UpdateJs_name_of_module_idet al. to generate.esmmodule-namesReferences:
This fixes #3383, and expands upon the work in #2037.