Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support .mts .cts #1564

Closed
wants to merge 3 commits into from
Closed

Conversation

bluelovers
Copy link
Contributor

@bluelovers bluelovers commented Dec 9, 2021

@cspotcode
Copy link
Collaborator

Like all PRs, this will need tests before we can merge it.

@bluelovers
Copy link
Contributor Author

i don't know how make test for this

@cspotcode
Copy link
Collaborator

cspotcode commented Dec 11, 2021

A proper test needs to use the feature that you're adding. So if you're adding support for new file extensions, then the test needs to use the file extensions to prove that ts-node behaves the way you want it to behave.

Most of ts-node's tests are very straightforward. They run the ts-node CLI and tell it to run code in a sample project. The code that spawns the CLI lives in ./src/test/**.spec.ts. The sample project lives in ./tests

Here's an example you can use as a starting point.
https://github.com/TypeStrong/ts-node/blob/main/src/test/index.spec.ts#L580-L584
https://github.com/TypeStrong/ts-node/tree/main/tests/main-realpath

This example tests something about symlinks. Obviously that's not relevant for your test. But the idea is the same: you have a sample project, and it gets invoked.

@cspotcode
Copy link
Collaborator

This is gonna need more work:

  • must register .js extension handler even if allowJs is off
    • necessary to compile .cjs, .cts, and to throw ESM error for .mts
  • consider conditionally registering .mjs extension when moduleType overrides are set
  • modify assertScriptCanLoadAsCJSImpl to understand that .mts overrides package.json
  • modify ESM loader to resolve from .mjs / .cjs to .mts / .cts, the same way it does for .js/.jsx/.ts/.tsx

And finally...

  • test coverage of the above

@cspotcode
Copy link
Collaborator

Might want to ship with #1361 so that require("./foo.cjs") resolves to require("./foo.cts")

@cspotcode
Copy link
Collaborator

Docs updates to go with this:

Update moduleTypes jsdoc and markdown: they say ts cannot use mts and cts; this is wrong

@cspotcode
Copy link
Collaborator

Rolling into #1694

@cspotcode cspotcode closed this Mar 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants