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

fix: package types #202

Merged
merged 2 commits into from Dec 19, 2023
Merged

fix: package types #202

merged 2 commits into from Dec 19, 2023

Conversation

userquin
Copy link
Member

@userquin userquin commented Nov 30, 2023

This PR includes:

  • use ./module.js in types.d.mts generation
  • move example to ESM-only

imagen

@@ -8,10 +8,21 @@
"types": "./dist/types.d.mts",
"default": "./dist/module.mjs"
},
"./utils": "./dist/utils.mjs"
"./utils": {
"types": "./dist/utils.d.mts",
Copy link
Member

Choose a reason for hiding this comment

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

is this needed? I think we can omit .mts for subpaths that match the types and typescript will infer them

Copy link
Member Author

Choose a reason for hiding this comment

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

missing typesVersions, Node10 complaining

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@@ -10,8 +10,16 @@
},
"./utils": "./dist/utils.mjs"
},
"main": "./dist/module.cjs",
"types": "./dist/types.d.ts",
"main": "dist/module.mjs",
Copy link
Member

Choose a reason for hiding this comment

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

I think anything that has the capability of running ESM will pick exports. What bug does this solve?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I mean, if we use .cjs Node10 will complain

Copy link
Member Author

Choose a reason for hiding this comment

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

imagen

imagen

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

@userquin userquin Nov 30, 2023

Choose a reason for hiding this comment

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

so, we need to move it to ESM-only or patch cjs with exports.default instead module.exports

Copy link
Member

Choose a reason for hiding this comment

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

It seems to me like this is just patching an issue that will never occur (because modules are loaded in Nuxt with jiti, not native ESM) and modules have type: module set anyway... 🤔

it might be time to remove the stub - cc: @pi0 who might have an idea whether it is still needed

Copy link
Member Author

@userquin userquin Nov 30, 2023

Choose a reason for hiding this comment

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

Copy link
Member Author

@userquin userquin Dec 1, 2023

Choose a reason for hiding this comment

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

@danielroe danielroe merged commit 40ef815 into nuxt:main Dec 19, 2023
1 check passed
@userquin userquin deleted the fix-mts-types-dts branch December 26, 2023 22:14
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