Hookyns / tst-reflect

Advanced TypeScript runtime reflection system
MIT License
338 stars 11 forks source link

ES Module support #54

Closed avin-kavish closed 2 years ago

avin-kavish commented 2 years ago

@Hookyns check this out. Tried to run under type: "module" and got require is not defined.

Hookyns commented 2 years ago

You can create a PR with small change here: https://github.com/Hookyns/tst-reflect/blob/53b56ac309e3f36c74c87d524268aa4084f87720/transformer/src/config.ts#L220-L229

Add the new module kind to line 228.

Hookyns commented 2 years ago

Published in tst-reflect-transformer@0.9.14. TY for the PR.

avin-kavish commented 2 years ago

Hmm, looks like it's still happening, https://stackblitz.com/edit/tst-reflect-issue-49-kdukpz

Get ctor is generated properly, but it adds this

const _ßr = require("tst-reflect");
avin-kavish commented 2 years ago

There's a require statement inserted in meta-writer/type-lib/TypeLibMetadataNodeGenerator.ts and meta-writer/inline/InlineMetadataNodeGenerator.ts

factory.createIdentifier("require"),

I think that's why.

Hookyns commented 2 years ago

There are some other places where are the requires generated, without using the isESMModule function.

But now I don't know if we modified the isESMModule correctly. It's not just the "module" option but combination with "moduleResolution" or package.json setting. I need to look into it a little bit.

avin-kavish commented 2 years ago

yeah, you might be right. I was asking about top-level await in the typescript repo and they pointed me to this.

https://www.typescriptlang.org/tsconfig#moduleResolution

It has to be set to nodenext apparently.

Hookyns commented 2 years ago

Should be fixed in tst-reflect@0.8.0 and tst-reflect-transformer@0.10.1.

Also there was some trouble with typelib mode in nodenext module, cuz of code related to ts-node. Hope it'll be okay.

avin-kavish commented 2 years ago
import { ExpressionKind, } from 'tst-expression';
         ^^^^^^^^^^^^^^
SyntaxError: Named export 'ExpressionKind' not found. The requested module 'tst-expression' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'tst-expression';
const { ExpressionKind, } = pkg;

Can you add a esm export to the packages?

avin-kavish commented 2 years ago

Oh look's like there is one issue. In es mode, imports have to use .js extension for relative imports. getCtor() will have to change. Currently, it's like import("./index")

Hookyns commented 2 years ago

@avin-kavish Do you test it with ttsc or ts-node?

I found some note - from another contributor solving issues for ts-node - saying that ts-node has some issues with extensions .js/.ts so we removed the extensions cuz of it. But it was before nodenext option. They have to fixed it cuz of nodenext IMHO.

avin-kavish commented 2 years ago

With ts-node. Not aware of any issues, i know it won't accept .ts extension though. Even .ts files have to be imported with .js.

Hookyns commented 2 years ago

Try tst-reflect-transformer@0.11.2, tst-reflect@0.11.0

avin-kavish commented 2 years ago

woot! works now.

avin-kavish commented 2 years ago

hmm... got another undefined getCtor() case

Hookyns commented 2 years ago

I'm working on it. 👍