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

[Feature] Support transformers written as ES modules #58

Closed
aomarks opened this issue Aug 21, 2021 · 8 comments · Fixed by #96
Closed

[Feature] Support transformers written as ES modules #58

aomarks opened this issue Aug 21, 2021 · 8 comments · Fixed by #96

Comments

@aomarks
Copy link

aomarks commented Aug 21, 2021

It is not currently possible to import a transformer that is written a standard ES module. Only transformers written as CommonJS modules work currently.

Node has supported standard ES modules since v12, so that's all currently supported versions of Node: https://nodejs.org/en/about/releases/. Node packages are increasingly being published as ES modules instead of CommonJS, so it would be great to be able to use them here too! One huge advantage of ES modules is that they can run directly both in Node and browsers.

https://nodejs.org/api/esm.html#esm_introduction

Note this doesn't require writing ts-patch itself in ES modules, since there is bi-directional interoperability in Node. ​To import an ES module from a CommonJS module, use const mod = await import("./some-module.js") instead of require. To check if a module should be imported as an ES module, look in its package.json for the {"type": "module"} field.

I see a new version is being written at #40 -- I think this would be an awesome thing to include in the rewrite.

Thanks!

@nonara
Copy link
Owner

nonara commented Aug 21, 2021

Thanks for the report. ESModule support is definitely important. I'm not sure if I'm missing something, but I'm not sure how you could implement this into CJS code without dramatic rewrites.

For example, the typescript compiler is a synchronous cjs file. In the case of tsc.js, you could probably make it work because you can pre-resolve the transformer files early (as you know exactly what tsconfig you're working with at execution, and you're only doing one). Even still, you'd still have to wrap the actual executable portion and only call it after you've resolved the promise for resolution.

In the case of the API (ie. typescript.js), it becomes much more difficult. You'd need to convert createProgram to an async function, and thus modify everything that leads to it, and the whole chain that use those, etc. Am I misunderstanding something? Is there a way to synchronously import via expression?

Edit:

I think the simple answer here is that typescript, itself, doesn't support esm transformers. Rewiring it to do so on our end wouldn't be a viable or sustainable route to take.

That said, however, our current support for .ts transformers is due to pre-transpiling. I think we could reasonably achieve esm support through the same means, by either:

  1. Detecting if transformer uses esm programmatically (via package.json if provided, or perhaps catch, if not)
  2. Adding a flag to indicate transformer is esm

Then, if it is necessary, we simply transpile ahead of time, like we do for .ts transformers

Again, I don't have a lot of knowledge around the actual esm spec or any recent developments thereof, so if there's anything I'm not seeing, I'd be glad to be wrong! From what I currently know, I think this may be the best means of achieving support.

@aomarks
Copy link
Author

aomarks commented Aug 22, 2021

Ah, interesting. It didn't occur to me that because you are patching TypeScript, it is not easy for you to add arbitrary asynchronous setup work. It is true that there is no synchronous dynamic import for ES modules. I can see that this would be quite hard to support!

I'm coming over from ttypescript (since it seems a bit abandoned). I'm guessing it might be easier to do this in the model it uses, because it's got an alternate frontend -- maybe it could just do arbitrary work before invoking any TypeScript APIs (in this case waiting for all the modules specified in compilerOptions.plugins.transform to be imported)?

@nonara
Copy link
Owner

nonara commented Aug 22, 2021

We're essentially doing the same thing as ttypescript, with the main differences being that we add a few more features and ours is done once, where tts does it each time it's loaded.

Both libraries patch ts.createProgram, which gets called from many places throughout the compiler. If we made that an async function, it would affect everything that calls it.

Actually just supporting tsc would be relatively simple. As you mentioned, it could do the resolution work before executing the compilation. It's easy for us to add that in. The issue is that we also support the TypeScript compiler API, which can be called multiple times, with different configs. But as I mentioned, the real issue is that the typescript library (which natively handles transformers) doesn't support esm transformers.

But, this is really just me thinking out loud. I don't think any of that is relevant to the ask. The right move is to add support further upstream, like we did to natively support .ts transformers.

Have you tried using --loader ts-node/esm as a node parameter (or specifying in NODE_OPTIONS env)?

@nonara nonara changed the title Support transformers written as ES modules [Feature] Support transformers written as ES modules Jan 6, 2023
@clearfram3
Copy link

Does TS 5.0 being mostly ESM now change this at all?

@nonara
Copy link
Owner

nonara commented Apr 20, 2023

This is now supported in v3

@nonara nonara mentioned this issue Jun 13, 2023
Merged
nonara added a commit that referenced this issue Jun 13, 2023
- Added Live Compiler (on-the-fly, in-memory patching)
- Added experimental ES Module support (closes #58)
- Added mutex locks (closes #75)
- Updated to support TS v5+ (closes #83 closes #93)
- Fixed patching for non-standard libraries (cannot guarantee they will work as expected in IDEs) (closes #85)
- Added caching
@colecrouter
Copy link

I'm on v3.0.2, and I'm unable to get ESM working.

plugins: "..." does not have an export "default": {}`.

It seems when loadPlugin is called, it tries to require(transformerPath). However, require does not support ESM as far as I know. It's also worth noting that transformerPath is "" anyway, so require returns {}. transformerPath is empty because it's assigned using require.resolve.

Unless I'm totally mistaken here, using import instead of require would fix the issue. However, that would require the use of async/await. I'm not sure about require.resolve, though, because using import.meta.resolve can only be called inside of a module, itself.

@nonara
Copy link
Owner

nonara commented Aug 18, 2023

@Mexican-Man Can you make a repro?

Esm requires the esm package, but it should warn you of that. The plugin path should be relative file path to the plugin file.

transformerPath should not be empty, as it's resolved from your file path in your plugin config entry in tsconfig. ESM wouldn't play a factor at that point.

Sounds like your config had an issue somewhere

@colecrouter
Copy link

@nonara It's more than likely I'm messing something up. Here's a reproduction with instructions. Please let me know if you spot anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants