Add support for ES modules #481
Conversation
bin/pegjs
Outdated
| if (args[0] !== "amd" && args[0] !== "commonjs" && args[0] !== "globals" && args[0] !== "umd") { | ||
| abort("Module format must be one of \"amd\", \"commonjs\", \"globals\", and \"umd\"."); | ||
| if (args[0] !== "amd" && args[0] !== "commonjs" && args[0] !== "esm" && args[0] !== "globals" && args[0] !== "umd") { | ||
| abort("Module format must be one of \"amd\", \"commonjs\", \"esm\", \"globals\", and \"umd\"."); |
There was a problem hiding this comment.
I think it would be slightly better to identify the format as es2015 — it is more readily understandable. But if most common tools that work with module formats use esm, then it would be probably better to keep it (I’d like to see examples of such tools then).
The condition also got a bit too long — maybe extract the list of formats into a const named e.g. MODULE_FORMATS and rewrite the test as MODULE_FORMATS.indexOf(args[0]) === -1? Up to you. Best done as a separate commit before the addition itself.
bin/pegjs
Outdated
| @@ -253,7 +253,7 @@ while (args.length > 0 && isOption(args[0])) { | |||
| } | |||
|
|
|||
| if (Object.keys(options.dependencies).length > 0) { | |||
| if (options.format !== "amd" && options.format !== "commonjs" && options.format !== "umd") { | |||
| if (options.format !== "amd" && options.format !== "commonjs" && options.format !== "esm" && options.format !== "umd") { | |||
There was a problem hiding this comment.
Same as above (es2015, maybe long condition → MODULE_FORMATS_WITH_DEPS).
|
@fatfisz Thanks! I just reviewed.
My philosophy is not doing parameter validation in functions/methods when I work in dynamic languages because it would pretty much lead to duplication of the (absent) type system if done consistently. On the other hand, a program shouldn’t crash or produce an exception because of bad user input and it should produce appropriate error messages for given interface, which logically leads to validation in
No. I think the right way to test this would be using integration tests — that is, compile a parser, load it in a suitable environment for given module format, and verify that the exports are there and they work. Setting this up is obviously a bit of work, so I chose not to do it given that all module-related code is self-contained and unlikely to break without directly touching it. But I would accept such tests as a contribution. For this PR this means relying on manual testing. |
|
About the name: Babel uses "es2015", Rollup uses "es", Webpack doesn't have the target yet, though the suggested name at the moment is "module". Also, I suggested "esm" based on what I've seen in the discussions about what should be the extension for ES modules in Node.js. Though "es" and "jsm" was also listed there. Out of those my personal preference is "es" - the same module system will be there in ES2017, ES2018, etc., so sticking to the version in this case seems a bit pointless, but I could be convinced otherwise. Still, the decision is not up to me ;) |
|
I think you convinced me. Having a year in the module format name may be important for projects that deal with multiple ECMAScript versions (like Babel or TypeScript), but not so much here. And it’s unlikely that the module system will ever change in a way that would justify using the year for versioning. Let’s go with |
|
Any news about this? I currently have some troubles making Rollup understand that the parser actually does export a |
|
This branch works great for me! |
|
@fatfisz Github is not letting me merge this due to conflicts in README.md, so if you can just fix those, I'm happy to merge it straight away |
|
Sure, will see if I can get it done today! |
Previously there was a very long condition.
|
Rebased + added a missing piece of info to |
|
@fatfisz Thanks, I've merged it now 😄 |
|
Hi folks, I suggest to add this feature information into the real official documentation of the project. |
Related to #423.
A couple of notes:
bin/pegjsscript, shouldn't it be in thegeneratemethod instead?This does not yet contain changes to README, which I will add later if the code is good first.