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

TypeScript issues #33

Open
philkunz opened this issue Jan 28, 2024 · 9 comments
Open

TypeScript issues #33

philkunz opened this issue Jan 28, 2024 · 9 comments
Labels
bug Something isn't working

Comments

@philkunz
Copy link

Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean './enums.js'?
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean './watcher_handler.js'?
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean './watcher_locker.js'?
Relative import paths need explicit file extensions in ECMAScript imports when '--moduleResolution' is 'node16' or 'nodenext'. Did you mean './watcher_poller.js'?

... and so on.

@fabiospampinato
Copy link
Owner

Maybe obvious, but how do I reproduce the issue? 🤔 Once compiled imported files seem to contain an extension already.

@fabiospampinato fabiospampinato added the needs more info Some more info is needed. label Apr 5, 2024
@fabiospampinato
Copy link
Owner

/cc @benmccann in case you know how to reproduce this problem.

@benmccann
Copy link
Contributor

@fabiospampinato if you do npm install watcher and then open up node_modules/watcher/dist/types.d.ts you'll see that the compiled output does not have file extensions:

Screenshot from 2024-04-10 10-04-04

With #38, the file extensions are present in the compiled output

@philkunz
Copy link
Author

Here are the compiler options to reproduce this:

"compilerOptions": {
"experimentalDecorators": true,
"useDefineForClassFields": false,
"target": "ES2022",
"module": "NodeNext",
"moduleResolution": "NodeNext",
"esModuleInterop": true,
"verbatimModuleSyntax": true
},

Until the fix is merged, there is a fixed version available at @tempfix/watcher, https://code.foss.global/tempfix/watcher

This error is present also in the dependencies of this package.

@SellersEvan
Copy link

Same issue here. I believe the extension name is required to be fully ESM compatible.

@benmccann
Copy link
Contributor

Yeah, it sounds like it's a bug in tsex, which is used to package this library: https://github.com/fabiospampinato/tsex

@fabiospampinato
Copy link
Owner

I finally had the time to look into this, but it's going to take a little longer to fix it. A brain dump:

  • This commit in tsex, the convenience library I'm using to compile my npm packages, should make sure imports in .d.ts files get rewritten to have an extension too, which is basically the root problem here.
  • There was an interesting comment in there though, that said: "// We don't actually want to fully resolve the path for declaration files", because I guess I had tried rewriting imports in declaration files already, but TS was complaining if I was importing from .d.ts files explicitly, but it wasn't complaining if I didn't specify the extension, so I just left those imports be. Apparently you need to say that you are importing from a .js file, then TS will resolve the .d.ts file by itself, you can't just tell it what the .d.ts file is, which doesn't make too much sense to me as far as "being explicit" goes, like it makes no sense to do an import type from a .js file to begin with, but whatever, I guess I know that now.
  • Switching the module compiler flag to node16 causes TS to no longer emit pure ESM code, so that's not happening, as emitting pure ESM only is what I want.
  • This explicit imports problem is kinda recursive, just updating tsex in watcher doesn't fix it because there's another type "error" like that in a dependency that watcher imports, so I need to go over all my packages and update them all to totally fix this issue.
  • Also I was doing an unrelated weird hack in tsex to make sure I wouldn't have to specify source and output directories in every tsconfig.json of every package I maintain, but TS v5.5.0 incidentally added a template variable that addresses this issue properly, so that's another good reason to go over all my packages and update them anyway, I guess.

So yeah I need to go over these packages, hopefully I can get it done by tomorrow, it may take a little longer than that. In general I'd recommend putting skipLibCheck: true in your tsconfig.json, not using it is kinda unworkable imo, every non-patch release of TS could have breaking changes, one can't expect all dependencies to pass all checks in whatever arbitrary version of TS you are using.

@benmccann
Copy link
Contributor

Switching the module compiler flag to node16 causes TS to no longer emit pure ESM code, so that's not happening, as emitting pure ESM only is what I want.

Oh, where is the output non-ESM with this flag? I hadn't noticed anything non-ESM with #38, but would be curious to learn more so I can keep an eye out for it in other projects I work on

@fabiospampinato
Copy link
Owner

fabiospampinato commented Jun 30, 2024

Oh, where is the output non-ESM with this flag?

Unfortunately I can no longer seem to be able to reproduce the issue 🙃 I don't know if I had done something wrong the other day or what.


This issue should be fixed by this commit (and doing the same thing for everything watcher depends on). I'll double-check after I make a release before closing this issue.

@fabiospampinato fabiospampinato added bug Something isn't working and removed needs more info Some more info is needed. labels Jun 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants