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

Add ts-node transpiler plugin #726

Closed
alangpierce opened this issue Jul 21, 2022 · 2 comments · Fixed by #729
Closed

Add ts-node transpiler plugin #726

alangpierce opened this issue Jul 21, 2022 · 2 comments · Fixed by #729

Comments

@alangpierce
Copy link
Owner

ts-node now allows a configurable transpiler and already has built-in support for swc. It would be great for Sucrase to provide plugin as well. I'm hoping to work on this soon, just filing for tracking purposes.

Docs are here: https://typestrong.org/ts-node/docs/transpilers/

For ts-node users, this would be a nice drop-in replacement to speed up the transpile step. I already prototyped a quick plugin and ran it through a stress test of a 1 million line codebase and confirmed that ts-node with Sucrase is almost 2x faster than ts-node with swc for handling the initial transpile/import. I'll make sure to include a reproducible benchmark as part of this work, similar to the benchmark already on the README.

For Sucrase users, this would be a more polished and feature-complete alternative to sucrase/register and sucrase-node, which are both fairly simple integrations using the pirates library. Most significantly, this would make it easy to use Sucrase for ESM in node via ts-node's loader hook. I've been investigating adding an ESM loader hook for Sucrase, but there are certainly some details to work through, and it would be nice to just integrate with a library that already handles it all. In addition to ESM support, ts-node also has a REPL and would allow configuration (e.g. JSX config) via tsconfig.json.

In terms of implementation, it looks like there are some compile modes that would need to be supported, e.g. targeting commonjs but preserving dynamic import(), so those are also options that should be added to Sucrase as part of this work.

Some open questions:

  • Should the integration be a separate package (living in the integrations directory) or part of the core sucrase package? It looks like it should be possible to write without taking on any additional dependencies. Making it part of sucrase seems ideal as a way to quickly get started.
  • Should this ts-node integration replace sucrase/register and sucrase-node (as a future breaking change)? My leaning is to keep Sucrase's integrations as a lighter-weight way of supporting the common case, but it may make sense to down-scope Sucrase to purely a transpile function, since that's where I've mostly focused anyway. There are also plenty of non-TypeScript cases (e.g. Flow or a Babel-like JSX/imports setup without tsconfig) that Sucrase supports.
  • Similarly, is it worth adding a built-in Sucrase loader hook when there's already a ts-node integration? My leaning is yes for similar reasons as above. But this may depend on the details of how complex a loader hook turns out to be.
alangpierce added a commit that referenced this issue Jul 25, 2022
Progress toward #726

This PR adds a new `preserveDynamicImport` flag that, when enabled, changes the
`imports` transform so that only static `import`s are transformed, while dynamic
`import()` expressions are passed through as-is.

This behavior matches what TypeScript does when handling a `.cjs` file when
`module: nodenext` is specified, and ts-node expects transpilers to handle this
mode, so this PR is a necessary step toward Sucrase as a ts-node plugin.

This mode also seems like the best interpretation of `import()` in new code
these days; if `import()` is transpiled, then it's impossible to access ESM
code from CJS. My tentative plan is to change this mode to be the default
behavior in an upcoming semver-major release.
alangpierce added a commit that referenced this issue Jul 25, 2022
Progress toward #726

This PR adds a new `preserveDynamicImport` flag that, when enabled, changes the
`imports` transform so that only static `import`s are transformed, while dynamic
`import()` expressions are passed through as-is.

This behavior matches what TypeScript does when handling a `.cjs` file when
`module: nodenext` is specified, and ts-node expects transpilers to handle this
mode, so this PR is a necessary step toward Sucrase as a ts-node plugin.

This mode also seems like the best interpretation of `import()` in new code
these days; if `import()` is transpiled, then it's impossible to access ESM
code from CJS in Node, so leaving it alone allows a mechanism to import
ESM-only libraries. My tentative plan is to change this mode to be the default
behavior in an upcoming semver-major release.
alangpierce added a commit that referenced this issue Jul 26, 2022
Progress toward #726

This is currently an opt-in flag handling a nuance in how TS transpiles imports
like these:
```ts
import foo = require('foo');
```
In the new nodenext mode when targeting ESM, TS now transforms this code to use
`createRequire`. The change is described here:
https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#commonjs-interoperability

This PR adds a flag `injectCreateRequireForImportRequire` to enable this
different behavior. It's unclear if this should eventually become the default,
but it at least seems useful as an option to avoid some ESM/CJS interop
frustration.

As I understand it, the main benefit of this change over explicit `createRequire`
is that it allows a single codebase to be transpiled to Node ESM and Node CJS
while using this syntax. A downside is that `createRequire` is Node-specific and
needs special support from bundlers, but it looks like webpack can recognize the
pattern. In most situations, real ESM `import` syntax is probably preferable,
but this syntax makes it possible to force the use of CJS.

The ts-node transpiler plugin system expects transpilers to have this mode as an
option, so this is a step closer to having a compliant ts-node plugin.
alangpierce added a commit that referenced this issue Jul 26, 2022
Progress toward #726

This is currently an opt-in flag handling a nuance in how TS transpiles imports
like these:
```ts
import foo = require('foo');
```
In the new nodenext mode when targeting ESM, TS now transforms this code to use
`createRequire`. The change is described here:
https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#commonjs-interoperability

This PR adds a flag `injectCreateRequireForImportRequire` to enable this
different behavior. I'm gating this behind a flag out of caution, though it's
worth noting that TS gives an error when using this syntax and targeting
module esnext, so it will likely be safe to switch to this new emit strategy as
the default behavior in the future.

As I understand it, the main benefit of this change over explicit `createRequire`
is that it allows a single codebase to be transpiled to Node ESM and Node CJS
while using this syntax. A downside is that `createRequire` is Node-specific and
needs special support from bundlers, but it looks like webpack can recognize the
pattern. In most situations, real ESM `import` syntax is probably preferable,
but this syntax makes it possible to force the use of CJS.

The ts-node transpiler plugin system expects transpilers to have this mode as an
option, so this is a step closer to having a compliant ts-node plugin.
@cspotcode
Copy link

Follow-up from my comment nodejs/node#43818 (comment)

Are you interested in getting sucrase support added as a dedicated "sucrase": true flag in ts-node? It can still live as a separate package, so you'll still have ownership of it. But is the dedicated flag a good thing for user experience, or no?

Right now, the swc transpiler integration lives with ts-node, so when someone sets "swc": true that's equivalent to "transpiler": "ts-node/transpilers/swc" Then that integration tries to require("@swc/core") The integration is bundled, but the swc compiler is a peer dependency.

For sucrase, could be a bit different: the integration can be part of the peer dependency, not bundled. But otherwise similar:
"sucrase": true is equivalent to "transpiler": "sucrase/ts-node" or wherever you decide it should live.

The next release of ts-node is going to be a major, so I can make breaking changes to the transpiler plugin API if we want. I've had to tweak the API over time, to support unanticipated issues with the swc integration. But till now, I've been the sole consumer of the API, at least as far as I'm aware. Any feedback you have is appreciated.

@alangpierce
Copy link
Owner Author

Hi @cspotcode, thanks for reaching out, and thanks for all the great work you've done on ts-node!

Yes, "sucrase": true or --sucrase would be wonderful, I was planning to ask about that. I like the general strategy of having easy-to-use recommended transpilers while also leaving it open for anyone else who wants to add their own.

Good question about where the integration should live and how to handle possible breaking changes both on the ts-node side and the Sucrase side. I'm planning on some upcoming breaking changes in Sucrase, so it seemed useful to have the integration live there, though if the plugin interface is going through a lot of breaking changes, it might be smoother for it to live on the ts-node side. Let's plan on it living in Sucrase for now, but can coordinate.

Overall the plugin interface has been pretty smooth. The fact that create is called 5 times was surprising at first, but also was helpful in understanding the nuances around how TS deals with .cts and .mts files. I have a few minor thoughts/questions on the plugin interface, and would be great to get your eyes on the details. I'll put up a draft PR of what I have so far and can tag you in some specifics.

alangpierce added a commit that referenced this issue Jul 27, 2022
Fixes #726

This makes it easy to use Sucrase with all of the nice benefits that ts-node
provides, such as an ESM loader, tsconfig discovery, and a REPL.

The implementation is based off of these docs:
https://typestrong.org/ts-node/docs/transpilers/
and the built-in SWC integration:
https://github.com/TypeStrong/ts-node/blob/main/src/transpilers/swc.ts

Some implementation notes:
* Currently, the plugin is written as a CJS module and written in JS rather than
  TS. This seems like the most reasonable initial approach since currently all
  build artifacts go to a `dist` directory, and it's important to not expose
  that fact in the usage. The future plan is to add `exports` config to
  package.json in a semver-major release, which should clean this up.
* The plugin is included as part of the core `sucrase` package rather than as an
  integration to be installed separately. This should make the usage and version
  management a little easier, and feels reasonable because the integration is
  small and has zero dependencies.
* I rewrote the usage section of the README to put installation instructions at
  the top and to suggest ts-node as the recommended way to use Sucrase with Node.
1Lighty pushed a commit to Astra-mod/sucrase that referenced this issue Aug 14, 2022
Progress toward alangpierce#726

This PR adds a new `preserveDynamicImport` flag that, when enabled, changes the
`imports` transform so that only static `import`s are transformed, while dynamic
`import()` expressions are passed through as-is.

This behavior matches what TypeScript does when handling a `.cjs` file when
`module: nodenext` is specified, and ts-node expects transpilers to handle this
mode, so this PR is a necessary step toward Sucrase as a ts-node plugin.

This mode also seems like the best interpretation of `import()` in new code
these days; if `import()` is transpiled, then it's impossible to access ESM
code from CJS in Node, so leaving it alone allows a mechanism to import
ESM-only libraries. My tentative plan is to change this mode to be the default
behavior in an upcoming semver-major release.
1Lighty pushed a commit to Astra-mod/sucrase that referenced this issue Aug 14, 2022
…erce#728)

Progress toward alangpierce#726

This is currently an opt-in flag handling a nuance in how TS transpiles imports
like these:
```ts
import foo = require('foo');
```
In the new nodenext mode when targeting ESM, TS now transforms this code to use
`createRequire`. The change is described here:
https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#commonjs-interoperability

This PR adds a flag `injectCreateRequireForImportRequire` to enable this
different behavior. I'm gating this behind a flag out of caution, though it's
worth noting that TS gives an error when using this syntax and targeting
module esnext, so it will likely be safe to switch to this new emit strategy as
the default behavior in the future.

As I understand it, the main benefit of this change over explicit `createRequire`
is that it allows a single codebase to be transpiled to Node ESM and Node CJS
while using this syntax. A downside is that `createRequire` is Node-specific and
needs special support from bundlers, but it looks like webpack can recognize the
pattern. In most situations, real ESM `import` syntax is probably preferable,
but this syntax makes it possible to force the use of CJS.

The ts-node transpiler plugin system expects transpilers to have this mode as an
option, so this is a step closer to having a compliant ts-node plugin.
alangpierce added a commit that referenced this issue Oct 5, 2022
Fixes #726

This makes it easy to use Sucrase with all of the nice benefits that ts-node
provides, such as an ESM loader, tsconfig discovery, and a REPL.

The implementation is based off of these docs:
https://typestrong.org/ts-node/docs/transpilers/
and the built-in SWC integration:
https://github.com/TypeStrong/ts-node/blob/main/src/transpilers/swc.ts

Some implementation notes:
* Currently, the plugin is written as a CJS module and written in JS rather than
  TS. This it the easiest approach with the current build system, though it may
  be reasonable to extend the build system to compile a `ts-node-plugin-src`
  directory to `ts-node-plugin` or something like that. The future plan is to add
  an `exports` line to `package.json` in a semver-major release, which will make
  this a bit easier to manage.
* The plugin is included as part of the core `sucrase` package rather than as an
  integration to be installed separately. This should make the usage and version
  management a little easier, and feels reasonable because the integration is
  small and has zero dependencies.
* I rewrote the usage section of the README to put installation instructions at
  the top and to suggest ts-node as the recommended way to use Sucrase with Node.
* I added an `integration-test` directory with various cases that this plugin should
  handle, and it may be useful for future node/tooling integration tests as well.
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 a pull request may close this issue.

2 participants