-
Notifications
You must be signed in to change notification settings - Fork 343
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(*): migrate rollup to tsup, support esm correctly, remove *.spec.* for dist #21
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments to share intention
package.json
Outdated
"dist/**/*", | ||
"esm/**/*", | ||
"*.d.ts" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tsup's default use only dist
], | ||
"publishConfig": { | ||
"access": "public", | ||
"main": "./dist/index.js", | ||
"module": "./dist/index.mjs", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
module field for yarn berry
https://yarnpkg.com/configuration/manifest#module
"import": "./esm/index.mjs", | ||
"require": "./dist/index.js" | ||
"import": { | ||
"types": "./dist/index.d.mts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*.d.mts is generated by tsup default
|
||
export default defineConfig({ | ||
format: ['cjs', 'esm'], | ||
entry: ['src/*.ts', 'src/*/*.ts', '!**/*.{spec,test,test-d}.*'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#19 !**/*.{spec,test,test-d}.*
: This prevent that dist contain test files
package.json
Outdated
"typescript": "^5.4.5", | ||
"vitest": "^1.5.2" | ||
}, | ||
"sideEffects": false, | ||
"scripts": { | ||
"prepack": "yarn build", | ||
"build": "rm -rf dist esm && tsc -p tsconfig.json --declaration --emitDeclarationOnly --declarationDir dist && rollup -c rollup.config.js && ./.scripts/postbuild.sh", | ||
"build": "tsup", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So simple
Fixed the Node10 resolution in the commit in 755b938. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution! I wish we had time to better configure the settings to generate declaration maps and make the chunk names reflect their actual names. (rather than chunk-*.mjs
files..)
Co-authored-by: Jonghyeon Ko <jonghyeon@toss.im>
Cool! I didn't think about these topics you suggest. Let's try it gradually |
close #19 close #20
@raon0211 In my opinion, This change is fit for your requirement. Review strictly please!