Lift the restriction that -bs-package-name is mandatory for -bs-package-output
#384
Conversation
jscomp/core/js_packages_info.ml
Outdated
| let bs_module_type_sentinel = "MELANGE_INVALID_PACKAGE_OUTPUT" | ||
|
|
||
| let add_npm_module_system (packages_info : t) module_system = | ||
| let m = { module_system; path = bs_module_type_sentinel; suffix = Js } in |
There was a problem hiding this comment.
I wonder if there could be any issues later on due to the path being set to this sentinel value, I guess we rely here on the constraint where Dune private libs can't be consumed from public ones, right? Or what's the path used for?
There was a problem hiding this comment.
This is a bit unfortunate, but it's here because I want to support 2 workflows, and I don't wanna break the CLI interface just yet.
- the flow where you emit (possibly multiple) JS files next to the
.cmj - the flow where those (possibly multiple) package specs are generated one JS file at a time (implies CMJ generation with
-bs-stop-after-cmj
There was a problem hiding this comment.
I think that, in the future, we can separate all these concerns into a better command line interface:
- "batch" mode gets (potentially multiple)
-bs-package-output commonjs:./folder:.bs.js - "separate compilation" mode gets
-bs-path-inside-package folder -bs-stop-after-cmjand-bs-module-type es6 -o foo.mjs
test/set-package-name-in-js.t
Outdated
| $TESTCASE_ROOT | ||
|
|
||
| $ cd output/app/ | ||
| $ melc $BSPKG -bs-package-output commonjs:lol -nopervasives -I ../../lib ../../app/b.cmj -o b.js |
There was a problem hiding this comment.
lol is the path to the module inside the "group" or package, right? I understand this only matters if the modules inside app would be consumed somewhere else. But shouldn't it be app?
There was a problem hiding this comment.
Actually, does the path in -bs-package-output matter at all when generating .js artifacts? I understand the "package paths" only matter for .cmj step.
There was a problem hiding this comment.
this was an oversight. it should be -bs-module-type instead.
| $ cd - | ||
| $TESTCASE_ROOT | ||
|
|
||
| $ cd app/ | ||
| $ melc $PKG_NAME $DUMMY_SPECS -I ../lib b.ml -nopervasives -bs-stop-after-cmj | ||
| $ melc $PKG_NAME -bs-package-output commonjs:./app:.js -I ../lib b.ml -nopervasives -bs-stop-after-cmj |
There was a problem hiding this comment.
Is it required to pass the .js extension to the cmjs? On the other test set-package-name-in-js, I see it's been removed.
There was a problem hiding this comment.
it's required for "batch compilation", i.e. output .js at same time as .cmj.
But not for "separate compilation" mode.
allows to fix es6 generation from separate compilation in a way that's easier to reason about
dereferencing package specs
…/melange into anmonteiro/no-bs-package-name
CHANGES:
- [melange] Introduce 2 explicit modes of JavaScript compilation:
- "Batch compilation": produces `.cmj` and `.js` files at the same time (this
is the previous behavior -- using `--bs-package-output
MODULE_SYSTEM:REL_PATH:JS_EXTENSION`)
- "Separate emission": produces _only_ `.cmj` files with `--bs-stop-after-cmj
--bs-package-output REL_PATH_ONLY`, and allows emitting JavaScript files
separately, with `--bs-module-type MODULE_SYSTEM -o
TARGET_FILE.JS_EXTENSION`
([melange-re/melange#384](melange-re/melange#384))
- [mel]: Fix `mel build --watch` exiting after the first change
([melange-re/melange#401](melange-re/melange#401))
- [melange]: Remove dependency on `reason`. Reason syntax users should install`
reason` from their preferred package manager instead, and Melange / Dune will
find it in `$PATH` ([melange-re/melange#409](melange-re/melange#409))
- [melange]: Remove dependency on `napkin` (the ReScript syntax parser). Users
that depend on libraries written in ReScript syntax should install the `mel`
package and Melange / Dune will find the `rescript_syntax` binary in `$PATH`
([melange-re/melange#411](melange-re/melange#411))
Also:
-bs-module-typeto simplify generatingJSfiles