Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: package types #202

Merged
merged 2 commits into from
Dec 19, 2023
Merged

fix: package types #202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
729f2b8
fix: package types
userquin Nov 30, 2023
8d051ae
chore: revert utils subpackage exports
userquin Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions example/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FalseExportDefault.md

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.

vite-pwa/nuxt#102 (comment)

Copy link
Member Author

@userquin userquin Nov 30, 2023 •
edited
Loading

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 •
edited
Loading

Choose a reason for hiding this comment

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

uhmmm, check these 3 for example, missing type module:

Copy link
Member Author

@userquin userquin Dec 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

check also antfu-collective/vite-plugin-inspect#100 and the PR antfu-collective/vite-plugin-inspect#101

"types": "dist/types.d.ts",
"typesVersions": {
"*": {
"*": [
"./dist/*",
"./*"
]
}
},
"files": [
"dist"
],
Expand Down
2 changes: 1 addition & 1 deletion src/commands/build.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ export type { ${typeExports[0].names.join(', ')} } from './module'

await fsp.writeFile(dtsFile, dtsContents, 'utf8')
if (!existsSync(dtsFileMts)) {
await fsp.writeFile(dtsFileMts, dtsContents, 'utf8')
await fsp.writeFile(dtsFileMts, dtsContents.replaceAll('./module', './module.js'), 'utf8')
}
}

Expand Down