Skip to content

Conversation

@stagas
Copy link

@stagas stagas commented Sep 21, 2023

Hey, thanks for the extension! Very useful, though I was trying out Deno and it requires specifiers to include the filename extension, so I added an option config.keepExtensions: boolean to do just that.

One thing to note, however, is that it will do a path.join(item.name, 'index.ts') when it is a folder, which can be a problem when it is an index.tsx, though I was unsure how to solve that cleanly, maybe you can help here if you need this.

It solves #11 :)

Cheers!

@espenja
Copy link

espenja commented Oct 8, 2023

Are you sure this solves #11 ?
The request is specifically for allowing the .js extension (not .ts) to be added to the filename when the package is targeting ESM.
Running node --loader ts-node/esm ./src/index.ts crashes if a barrel file contains an import statement ending in .ts (as well as crashing when the import statement only imports with the folder syntax)

@stagas
Copy link
Author

stagas commented Oct 9, 2023

Files ending in .ts work in deno and esbuild and it should work for ESM as it keeps all extensions although i haven't tested that scenario.

Make sure you have the "allowImportingTsExtensions": true, in tsconfig.json compilerOptions before testing with the node loaders.

let filePath: string;
if (keepExtensions) {
filePath =
item.type === "folder" ? path.join(item.name, "index.ts") : item.name;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is this issue it will default to index.ts if it's importing a folder in the barrel, so perhaps somehow it needs to work for .js and other extensions. Not sure what is the right solution here, maybe a configuration or else check the file system and try all extensions until one succeeds. There might be a lib for that.

@espenja
Copy link

espenja commented Oct 9, 2023

Files ending in .ts work in deno and esbuild and it should work for ESM as it keeps all extensions although i haven't tested that scenario.

Make sure you have the "allowImportingTsExtensions": true, in tsconfig.json compilerOptions before testing with the node loaders.

I didn't know about that option, pretty neat!
Although this works for (some?) bundlers, I think it may add restrictions on how your tsconfig must be setup. For example, I don't believe this will work if you are using tsc to emit files.
From the docs of allowImportingTsExtensions:

This flag is only allowed when --noEmit or --emitDeclarationOnly is enabled, since these import paths would not be resolvable at runtime in JavaScript output files. The expectation here is that your resolver (e.g. your bundler, a runtime, or some other tool) is going to make these imports between .ts files work.

The intellisense in VSCode when hovering over that option also has additional requirements to nodeResolution, but I don't know which of these, or if both, are correct:

Allow imports to include TypeScript file extensions. Requires '--moduleResolution bundler' and either '--noEmit' or '--emitDeclarationOnly' to be set.

Maybe the config option could be changed to either keep extension or allow for more customizability by allowing the user to prefer a specific file extension being used?

@0x80
Copy link

0x80 commented Nov 21, 2023

I am a little confused. Why would we want the actual file extension instead of just supporting the ESM standard?

Deno uses ESM and although it might allow for .ts to be used in imports, it should work with .js because that's what ESM requires.

So the problem to solve I think is "make this vscode extension compatible with the ESM standard".

Besides appending .js it should also resolve imports to /index.js for sub-folders that have an index/barrel file.

Ideally, the solution should look at the closest package manifest, and if "type": "module" is set, it should use the file extensions on import, because that is what defines an ES module. So there should be no need for an extra config option I think. But we could start with a configuration flag to keep it simple.

I have different priorities at the moment, but I will try to submit a PR for this in the near future.

@estruyf
Copy link
Owner

estruyf commented Jul 2, 2024

Ideally, the solution should look at the closest package manifest, and if "type": "module" is set, it should use the file extensions on import, because that is what defines an ES module. So there should be no need for an extra config option I think. But we could start with a configuration flag to keep it simple.

@0x80 I like this idea, that would be a great enhancement for the extension.

@0x80
Copy link

0x80 commented Jul 3, 2024

Since TS supports moduleResolution "bundler" I started using that and no longer use the import extensions, as all my larger projects that use barrel files also use bundlers for all code. In other words, don't count on me to submit a PR ;)

@estruyf
Copy link
Owner

estruyf commented Jul 3, 2024

No problem, @0x80; I'll check if I can start from what you already created. Thanks 🙏

@sharky98
Copy link

sharky98 commented Sep 9, 2024

To add the functionality in a pretty quick manner, what about having an choice option "Use file extension": ".js", ".js" and "none". That way the use could do whatever pleases him.

Also, the idea of reading the package.json or tsconfig.json might be a wrongful good idea. I say that because there is so many compilers, bundlers, type checkers in the wild, that you never know what a given user needs. Some can be configured to understand .ts as .js, some needs to deviate from ESM standard and not use an extension, etc.

This feature is the only thing that prevent me to use the watch mode of this plugin! 😊

@estruyf
Copy link
Owner

estruyf commented Sep 12, 2024

Thanks @sharky98 for the suggestion. I have included the changes for the manual way. For the future, it would still be helpful to have it automatically as well.

It might be useful to start a new issue for that and list all the criteria when certain file extensions need to be included.

An in-between option could be to show a pop-up with the question: Hey, we notice you're writing a module. Would you like to add the js as the file extension for your exports?

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 this pull request may close these issues.

5 participants