-
Notifications
You must be signed in to change notification settings - Fork 70
tsup #644
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
tsup #644
Conversation
hildjj
commented
Jul 17, 2025
- Switch to tsup for building. Fixes Switch to tsup #641.
- Switch to files approach so I don't have to maintain .npmignore
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #644 +/- ##
=======================================
Coverage 99.48% 99.48%
=======================================
Files 34 34
Lines 4283 4283
=======================================
Hits 4261 4261
Misses 22 22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
A few changes needed.
.bs-config.js
Outdated
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.
This file is left over from a previous web server that we used for docs.
@@ -5,6 +5,5 @@ module.exports = { | |||
"reject": [ | |||
"chai", // Moved to es6 | |||
"@types/chai", // Should match chai, | |||
"rimraf", // Requires node 20 |
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.
We're node 20+ now, no need to keep rimraf back at a previous version. Chai still needs to be held back until either we switch to modules or I try testing it with require(mjs), which should work in node 20.
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.
require(mjs) should work, except for jest. TODO: remove Jest, which is a problem for another day.
.npmignore
Outdated
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.
Moved to files: in package.json, so we can stop maintaining this brittle file.
benchmark/index.css
Outdated
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.
Left over from old express server. Now tested from docs with eleventy as the server.
benchmark/index.html
Outdated
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.
Left over from old express server. Now tested from docs with eleventy as the server.
test/all.js
Outdated
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.
Needed so tsup can bundle all of the tests. Wish there was a better way.
tsup.config.js
Outdated
platform: "browser", | ||
minify: true, | ||
outDir: "browser", | ||
target: "es5", |
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.
This is negotiable. Left in for now to generate a bunch of polyfills into the output.
tsup.config.js
Outdated
entry: { | ||
"test-bundle.min": "test/all.js", | ||
}, | ||
format: ["umd"], |
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.
This doesn't have to be umd. iife would work.
bundle: true, | ||
clean: false, | ||
splitting: false, | ||
external: ["mocha", "chai"], |
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.
note: do not add peggy here. The tests use require's deep into the project structure, so the tests and all of peggy have to be bundled together.
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.
The benchmarks serve as spot checks of the generated code now.