rules_nodejs icon indicating copy to clipboard operation
rules_nodejs copied to clipboard

ESM support

Open Toxicable opened this issue 3 years ago • 3 comments

🚀 feature request

Relevant Rules

  • ts_project
  • nodejs_binary
  • jasmine_node_test Maybe others, but these are the ones I'm familiar with

Description

  • Add support for executing JS that is running in ESM format (nodejs_binary, jasmine_node_test)
  • Add support for compiling to ESM (ts_project) (if needed)

Describe alternatives you've considered

I don't believe there's alternate ways to run ESM JS without core support in rules_nodejs


The primary motivation for this feature is for support of Angular v13 which only publishes ESM JS. The general rule with ESM is that an ESM file can import CJS or ESM, and CJS can require CJS and import ESM only via the import() function. Therefore you cannot require() ESM from a CJS file. This means that a codebase which runs a jasmine_node_test as it stands is unable to import Angular v13 (or any other ESM only packages)

(please correct me if this is wrong) My understanding of how NodeJS picks what module format to run a file in is determined by: First it will look at file extention: .cjs -> CJS, .mjs -> ESM If the extension is .js then it looks at the package.json field "type", where module -> ESM, commonjs -> CJS See "ESM_FILE_FORMAT" in the NodeJS docs for a detailed algorithm

With the above in mind I think the changes required to support ESM with nodejs_binary would be to rename internal JS files (node_patches.js, jasmine_runner.js) to be *.cjs, with the exception of loader.js which will need to be modified to be able to import() the users entry_point

For ts_library I think the most unambiguous method would be to have it output .mjs files, however that's not currently supported and there's a lot of discussion, see some of the threads around here https://github.com/microsoft/TypeScript/issues/18442 Therefore I think the next best option is just to output .js files then the user will need to ensure they have a package.json with type=module in order to declare the output to be ESM, this will need some docs. That would mean no changes are needed to the rule itself.

NodeJS docs: https://nodejs.org/api/esm.html

Toxicable avatar Jan 24 '22 00:01 Toxicable

I order to support jasmine_node_test the spec files need to be .mjs in order for the jasmine spec loader to be able to import it correctly. Ref https://github.com/jasmine/jasmine-npm/blob/main/lib/loader.js#L13 This could be done by adding a module attribute to the ts_project then calling a simple rule to rename outputs when module=esm (esm is just placeholder, values tbd). The attribute would be required when typescript adds support for .mjs outputs anyway since we'd need to pre-declare the .mjs outputs. Alternatively we could suggest that the rename rule is declared and called from the users codebase to reduce the maintenance overhead of rules_nodejs

Toxicable avatar Jan 25 '22 20:01 Toxicable

This issue has been automatically marked as stale because it has not had any activity for 6 months. It will be closed if no further activity occurs in 30 days. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

github-actions[bot] avatar Jul 28 '22 03:07 github-actions[bot]

Any progress on ESM support? Is required in Angular 13

fabianlema avatar Jul 31 '22 12:07 fabianlema

This issue has been automatically marked as stale because it has not had any activity for 6 months. It will be closed if no further activity occurs in 30 days. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

github-actions[bot] avatar Feb 01 '23 02:02 github-actions[bot]

This issue was automatically closed because it went 30 days without any activity since it was labeled "Can Close?"

github-actions[bot] avatar Mar 03 '23 04:03 github-actions[bot]