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

Add extensions to imports #7

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

sachaw
Copy link

@sachaw sachaw commented Apr 23, 2023

As per the Node ESM spec, file extensions are always required for relative imports, ref: https://nodejs.org/api/esm.html#terminology
I'm working on a better solidjs solution and this is currently a blocker for me

@rektdeckard
Copy link
Member

I'm not sure how this PR will produce working code...the files ARE typescript files, and do not have a .js extension. Seems to me that this needs to happen at the bundler stage, not in the source. Is this working for you?

@rektdeckard
Copy link
Member

rektdeckard commented Apr 24, 2023

Also, it seems irrelevant, because our bundle produces a single .mjs and .umd.js file, specified as module and main respectively in package.json. There is only a single file as far as consumers are concerned.

https://www.npmjs.com/package/@phosphor-icons/core?activeTab=code

@rektdeckard
Copy link
Member

I have not had problems using @phosphor-icons/core in solidjs. What exactly is the nature of your issue?

@sachaw
Copy link
Author

sachaw commented Apr 24, 2023

Thanks for the reply, I'll try and address all of the points:
as for how this will produce working code: Typescript has deemed it out of scope to add file extensions onto imports during transpiration, so it has to be done to the source. Typescript will interoperate the .js extension correctly. this change only impacts the generation of the .d.ts files (aswell as making the source more ESM compliant).

The other reason this is important is because somebody wishing to consume she source files in there project (thing having this as a submodule and not a npm dependancy) will also be scoped out of using ESM.

@rektdeckard
Copy link
Member

I don't follow...if you're using the source, then you're using TypeScript, and you don't need the extensions. If you're not using the source, then there are single-file bundles for ESM and CJS. I'm curious to see how you're using it that is not working?

@sachaw
Copy link
Author

sachaw commented Apr 26, 2023

One specific example is when using moduleResolution set to node16 or nodenext, even type definitions with relative imports without file extensions won't be resolved.

@rektdeckard
Copy link
Member

If you have a specific issue you can point me to to test and see the failure with my own eyes, I'd appreciate it. I am struggling to understand when and what would fail, and I have never seen anyone write Typescript code referencing Typescript code by a .js extension.

@sachaw
Copy link
Author

sachaw commented Apr 26, 2023

I'll prep an example when I have time, but this is some reading on the topic and should convey the reasoning (this also applies for .d.ts files)
https://www.typescriptlang.org/docs/handbook/esm-node.html

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.

2 participants