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 "exports" field to support newer module resolutions like node16 #1007

Merged
merged 1 commit into from Nov 16, 2022

Conversation

samuelstroschein
Copy link
Contributor

Setting the moduleResolution in the tsconfig to node16 results in no types being resolved.

I suppose the reason is a lacking exports field in package.json. See https://devblogs.microsoft.com/typescript/announcing-typescript-4-5-beta/#packagejson-exports-imports-and-self-referencing. I haven't tested it but expect the exports field to solve the bug + it's a good idea anyways.

With node16 module resolution (the new module resolution)
image

With nodenext module resolution (the default)

image

Here is a reproduction https://stackblitz.com/edit/typescript-3exolv?file=index.ts

@vercel
Copy link

vercel bot commented Nov 15, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
shoelace ✅ Ready (Inspect) Visit Preview Nov 15, 2022 at 4:21PM (UTC)

@samuelstroschein
Copy link
Contributor Author

samuelstroschein commented Nov 15, 2022

On that note, shoelace requires node v14. The exports field is supported since node 12. It might be a good idea to get rid of the main, module and types field in package.json as they should be redundant.

image

https://nodejs.org/api/packages.html#packages_exports

@claviska
Copy link
Member

Looks good to me. Thanks for jumping on this!

Did you want to update the PR to remove main | module | types as well? If not, I'm happy to handle it later today or tomorrow.

@samuelstroschein
Copy link
Contributor Author

@claviska I refrained from updating the PR to remove main, module, types to not possibly break some things. I don't expect anything to break but propose that you remove those keys and see if everything works.

@samuelstroschein
Copy link
Contributor Author

Whups just crossed my mind: The minimum TypeScript version would be 4.5 if types is removed.

@claviska claviska merged commit 0e77d8a into shoelace-style:next Nov 16, 2022
@claviska
Copy link
Member

Nice catch. I'll leave types but remove main and module then.

claviska added a commit that referenced this pull request Nov 16, 2022
@samuelstroschein
Copy link
Contributor Author

It would be helpful if you could release a new version. Otherwise, users of shoelace can't use moduleResolution: node16.

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.

None yet

2 participants