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

Support for TypeScript files #5

Closed
ifiokjr opened this issue Jun 9, 2022 · 13 comments · Fixed by #6
Closed

Support for TypeScript files #5

ifiokjr opened this issue Jun 9, 2022 · 13 comments · Fixed by #6

Comments

@ifiokjr
Copy link
Contributor

ifiokjr commented Jun 9, 2022

Description

Is it possible to add builtin support for .ts, .mts and .cts files?

I'm using this library and running tests on a TypeScript monorepo codebase. During development the "exports" field entrypoints redirect to TypeScript files via symlinks. This breaks with the following error.

ERR_UNKNOWN_FILE_EXTENSION - Unknown file extension ".ts" for X

I'm happy to open a PR and add the extensions, if you think it's a good idea.

/** @type {Record<string, string>} */
const extensionFormatMap = {
// @ts-expect-error: hush.
__proto__: null,
'.cjs': 'commonjs',
'.js': 'module',
'.json': 'json',
'.mjs': 'module'
}

@nicolo-ribaudo
Copy link

Does the built-in Node.js import.meta.resolve correctly resolve them, or does it throw?

@wooorm
Copy link
Owner

wooorm commented Jun 9, 2022

Can you make a small reproduction?

I did indeed notice the introduction of the error when I ported the code back two weeks ago, and checked that it’s thrown for a file such as readme.md, which I guess makes sense. Not sure if its intentional for these files though.

ifiokjr added a commit to ifiokjr/import-meta-resolve-ts that referenced this issue Jun 9, 2022
@ifiokjr
Copy link
Contributor Author

ifiokjr commented Jun 9, 2022

Here's the reproduction https:/ifiokjr/import-meta-resolve-ts

import.meta.resolve does correctly resolve TypeScript files.

@ifiokjr
Copy link
Contributor Author

ifiokjr commented Jun 9, 2022

Thinking beyond my short-term needs, it probably makes sense to remove the extension restrictions.

There are many file extensions which can legitimately be resolved. .wasm and .node spring to mind. There's even a case for allowing .md, .css etc... to be resolved as they can be loaded and used in the context of the code resolving the file path.

Would it make sense to remove the file extension restriction entirely?

@wooorm
Copy link
Owner

wooorm commented Jun 9, 2022

I don’t think so. Node has that map for some reason. But it doesn’t fail on this case. We also have that map, but it fails. The problem is not that there is a map. The problem is the divergence from Node

@wooorm wooorm closed this as completed in #6 Jun 11, 2022
wooorm pushed a commit that referenced this issue Jun 11, 2022
Closes GH-5.
Closes GH-6.

Reviewed-by: Titus Wormer <[email protected]>
Reviewed-by: Nicolò Ribaudo <[email protected]>
@thetutlage
Copy link

@wooorm I also expected it work, especially when using a TypeScript specific loader like ts-node.

If index.ts attempts to resolve ./foo.ts, then it will work with import.meta.resolve and not this package.

node --loader=ts-node/esm index.ts

@wooorm
Copy link
Owner

wooorm commented Jan 26, 2023

This issue is solved.

@thetutlage
Copy link

thetutlage commented Jan 26, 2023

I still face the similar issue with the latest version. Lemme try to share a reproduction repo

@thetutlage
Copy link

Here you go. https:/thetutlage/imr-issue

I see the issue is when the file has .js extension. However, TypeScript forces using .js file extension for TypeScript files as well. So it cannot be changed to .ts

@wooorm
Copy link
Owner

wooorm commented Jan 26, 2023

You are talking about loaders. Unrelated to this issue. More related to #10.
Loaders are implemented above this function: #10 (comment). They require tons of extra code to implement. But they can’t even be implemented: they’d require CLI arguments. You’d have to pass {loader: 'ts-node/esm'} to import-meta-resolve. But that’s a different API from resolve.

If you are going to use experimental APIs (loaders), you can just as well use import.meta.resolve itself?

@thetutlage
Copy link

Thanks for explaining!

Maybe, I had a wrong expectations from the package. I thought it is a drop-in polyfill for import.meta.resolve and addresses the use case where loaders take over the import URLs and resolve them.

But, it is fine, if you think that this use case is out of scope of this package. I just wanted to highlight my use case :)

@wooorm
Copy link
Owner

wooorm commented Jan 26, 2023

For your second paragraph: take a look at the readme, the differences section, I guess loader can be added there in the CLI flags (if it isn't already) But that gives an indication of how giant the scope of Node is! I think by the point all those flags are implemented, you almost have a copy of Node!

I think it would be cool to have some of these, particularly loaders, but that would probably be a new optional API (as CLI args have to be parsed, so it has to be Node that's executed from the CLI). And it would likely be a lot of work, which I'm not interested in currently doing

@thetutlage
Copy link

Yup, makes sense! Also, for a while (until it is experimental), the resolve API will be a moving target, so it might be wiser to cherry pick which features are worth implementing.

Anyways, thanks for your time :)

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.

4 participants