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.

Sign up for GitHub

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.json): Provide correct types for moduleResolution: node, node10, and node16 #16

Merged
merged 11 commits into from
Jun 3, 2024
Merged

fix(package.json): Provide correct types for moduleResolution: node, node10, and node16 #16

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f8534a1
fix: remove unnecessary exports overriding
manudeli May 31, 2024
ab2f0a3
Merge branch 'main' into fix/exports/remove-overriding
manudeli Jun 1, 2024
faf60b4
Merge branch 'main' into fix/exports/remove-overriding
manudeli Jun 1, 2024
3281a7a
Revert "fix: remove unnecessary exports overriding"
manudeli Jun 1, 2024
4948f2e
chore(package.json): add types field in publishConfig.exports
manudeli Jun 1, 2024
a14bcb1
build: Support moduleResolution: node10 and node16 (nodenext)
raon0211 Jun 3, 2024
8d96b0e
build: Support moduleResolution: node10 and node16 (nodenext)
raon0211 Jun 3, 2024
a00ed8d
build: Support moduleResolution: node10 and node16 (nodenext)
raon0211 Jun 3, 2024
c2b2a47
fix delay
raon0211 Jun 3, 2024
7974c90
Update .gitignore
raon0211 Jun 3, 2024
4987ef9
Update .scripts/postbuild.sh
raon0211 Jun 3, 2024
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
4 changes: 3 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,6 @@ coverage
dist
esm
.junit
out
out
*.d.ts
*.tgz
6 changes: 6 additions & 0 deletions .scripts/postbuild.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
echo "export * from './dist/array';" > array.d.ts
echo "export * from './dist/function';" > function.d.ts
echo "export * from './dist/math';" > math.d.ts
echo "export * from './dist/object';" > object.d.ts
echo "export * from './dist/predicate';" > predicate.d.ts
echo "export * from './dist/promise';" > promise.d.ts
14 changes: 12 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,36 +18,46 @@
},
"files": [
"dist/**/*",
"esm/**/*"
"esm/**/*",
"*.d.ts"
],
"publishConfig": {
"access": "public",
"main": "./dist/index.js",
"types": "./dist/index.d.ts",
"exports": {
".": {
"types": "./dist/index.d.ts",
"import": "./esm/index.mjs",
"require": "./dist/index.js"
Comment on lines +30 to 32
Copy link
Member Author

@manudeli manudeli Jun 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

However it is notoriously difficult to fix the "masquerading as CJS" part; I guess this should be fixed with either we integrate tshy (which is not PnP ready) or we go ESM only (after the release of Node.js 22).

In my opnion, this may fix our errors. I learned this way fixing from maintaining @suspensive/react to support cjs, esm for all environments

Suggested change
"types": "./dist/index.d.ts",
"import": "./esm/index.mjs",
"require": "./dist/index.js"
"import": {
"types": "./dist/index.d.ts",
"default": "./esm/index.mjs"
},
"require": {
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
}

Copy link
Collaborator

@raon0211 raon0211 Jun 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Actually it doesn't, because in order to be a "ESM" declaration,

  • The type's extension should be .mts (and TypeScript's cli does not support changing extensions of declarations)
  • And for each type declaration (e.g. in ./esm/index.d.mts), we have to specify extensions in its type.
-export * from './array';
+export * from './array/index.mjs';

However this is notoriously difficult in current TypeScript environments unless we introduce some good libraries like tshy. Meanwhile tshy does not support Yarn PnP.

Copy link
Member Author

Choose a reason for hiding this comment

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

then How about using tsup? I think tsup support this quite easily!
https://github.com/suspensive/react/blob/main/packages/react/package.json#L28-L36

Copy link
Member Author

@manudeli manudeli Jun 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@raon0211 Fast merging to fix TypeScript error and next Pull Request for tsup is also good for me. If you agree tsup way, I'll make new Pull Request for this!

Copy link
Collaborator

@raon0211 raon0211 Jun 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Actually tsup does not support our usecase neither.

In order to be tree-shaken well, it is necessary for us to use code splitting. However I find that tsup (and its internal implemenation, esbuild) does not support code splitting properly as rollup's preserveModules.

For example, while rollup completely splits our JavaScript files like this:
image

esbuild's splitting option only makes some chunks and does not split our project files completely.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we might try just using tsup's declaration generation...

},
"./array": {
"types": "./dist/array/index.d.ts",
"import": "./esm/array/index.mjs",
"require": "./dist/array/index.js"
},
"./function": {
"types": "./dist/function/index.d.ts",
"import": "./esm/function/index.mjs",
"require": "./dist/function/index.js"
},
"./math": {
"types": "./dist/math/index.d.ts",
"import": "./esm/math/index.mjs",
"require": "./dist/math/index.js"
},
"./object": {
"types": "./dist/object/index.d.ts",
"import": "./esm/object/index.mjs",
"require": "./dist/object/index.js"
},
"./predicate": {
"types": "./dist/predicate/index.d.ts",
"import": "./esm/predicate/index.mjs",
"require": "./dist/predicate/index.js"
},
"./promise": {
"types": "./dist/promise/index.d.ts",
"import": "./esm/promise/index.mjs",
"require": "./dist/promise/index.js"
},
Expand Down Expand Up @@ -81,7 +91,7 @@
"sideEffects": false,
"scripts": {
"prepack": "yarn build",
"build": "rm -rf dist esm && tsc -p tsconfig.json --declaration --emitDeclarationOnly --declarationDir dist && rollup -c rollup.config.js",
"build": "rm -rf dist esm && tsc -p tsconfig.json --declaration --emitDeclarationOnly --declarationDir dist && rollup -c rollup.config.js && ./.scripts/postbuild.sh",
"test": "vitest run --coverage --typecheck"
}
}
2 changes: 1 addition & 1 deletion src/promise/delay.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ describe('delay', () => {
await delay(100);
const end = performance.now()

expect(end - start).greaterThanOrEqual(100)
expect(end - start).greaterThanOrEqual(99)
});
})