-
-
Notifications
You must be signed in to change notification settings - Fork 11
Added option to keep file extensions. #24
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
base: master
Are you sure you want to change the base?
Conversation
|
Are you sure this solves #11 ? |
|
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 |
| let filePath: string; | ||
| if (keepExtensions) { | ||
| filePath = | ||
| item.type === "folder" ? path.join(item.name, "index.ts") : item.name; |
There was a problem hiding this comment.
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.
I didn't know about that option, pretty neat!
The intellisense in VSCode when hovering over that option also has additional requirements to
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? |
|
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 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. |
@0x80 I like this idea, that would be a great enhancement for the extension. |
|
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 ;) |
|
No problem, @0x80; I'll check if I can start from what you already created. Thanks 🙏 |
|
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 This feature is the only thing that prevent me to use the watch mode of this plugin! 😊 |
|
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 |
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: booleanto 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 anindex.tsx, though I was unsure how to solve that cleanly, maybe you can help here if you need this.It solves #11 :)
Cheers!