rules_nodejs icon indicating copy to clipboard operation
rules_nodejs copied to clipboard

ts_project ESM support

Open Toxicable opened this issue 2 years ago • 6 comments

PR Checklist

  • [x] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been added / updated (for bug fixes / features)

PR Type

  • [x] Feature (please, look at the "Scope of the project" section in the README.md file)

What is the current behavior?

Issue Number: #3277

What is the new behavior?

Adds ESM support in for ts_project

Does this PR introduce a breaking change?

  • [x] No - Only changes are internal API's

Toxicable avatar Jan 27 '22 22:01 Toxicable

@Toxicable Thank you working on this, I'm also strugling with the same problem.

Your implementation introduces on a new .mts file extension. The fact that Node.js relies on a .mjs file extension to detect whether to use JavaScript module syntax is a unique feature of Node.js only. In my opinion it's a bad design choice by the Node.js team. By introducing a new .mts file extension, Node.js behaviour would spill over into the TypeScript world. I'm not sure whether this is the right approach to resolve the issue. File extensions are ignored by web browsers and by Deno too.

realtimetodie avatar Apr 11 '22 06:04 realtimetodie

Your implementation introduces on a new .mts file extension

@diceride Unfortunately the ship has sailed and this is the deisgn that has been chosen by both NodeJS and the TypeScript Teams. I'm not keen to introduce any new designs here and would prefer to stick to what has already decided on.
The goal of this PR is to support the same set of file extensions that are already in TypeScript as outlined here https://www.typescriptlang.org/docs/handbook/esm-node.html#new-file-extensions In saying that, there's still the option to use the package.json#type=module method if that helps

Toxicable avatar Apr 11 '22 07:04 Toxicable

@diceride you might be confused by layering - this project doesn't make any decisions about how NodeJS works. We merely mirror their semantics into Bazel rules. Same for TypeScript. So like @Toxicable says, if the TypeScript docs describe behaviors related to understanding file extensions, Bazel just needs to expose those to the user of ts_project.

alexeagle avatar Apr 11 '22 14:04 alexeagle

CI failure

error TS18003: No inputs were found in config file 'C:/b/makknmlb/execroot/build_bazel_rules_nodejs/packages/typescript/test/ts_project/esm/tsconfig.json'. Specified 'include' paths were '["**/*"]' and 'exclude' paths were '[]'.

is probably because of Windows lack of sandboxing by default - do you need help to make it green?

alexeagle avatar Apr 11 '22 14:04 alexeagle

@alexeagle yeah that would be awesome if you got the time

Toxicable avatar Apr 11 '22 19:04 Toxicable

Note: https://github.com/bazelbuild/rules_nodejs/pull/3405 seems like the prerequisite but needs someone with some time to green it up...

alexeagle avatar Apr 25 '22 18:04 alexeagle

This Pull Request 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 Oct 26 '22 03:10 github-actions[bot]

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

github-actions[bot] avatar Nov 26 '22 02:11 github-actions[bot]