Build packages with unbuild to improve CJS support #2310
Conversation
🦋 Changeset detectedLatest commit: a340c6d The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for openapi-ts canceled.
|
size-limit report 📦
|
drwpow
force-pushed
the
unbuild
branch
7 times, most recently
from
4799af3 to
f375ba6
Compare
| "default": "./dist/index.js" | ||
| }, | ||
| "require": { | ||
| "types": "./dist/cjs/index.d.cts", |
There was a problem hiding this comment.
With separate .d.cts and .d.mts declarations generated, we don’t need to declare types separately. This was a little bit of a hack from previous builds that stuck around. With unbuild’s better generation in general, TypeScript can automatically locate types.
| @@ -8,19 +8,12 @@ | |||
| }, | |||
| "license": "MIT", | |||
| "type": "module", | |||
| "main": "./dist/index.js", | |||
| "module": "./dist/index.js", | |||
There was a problem hiding this comment.
"module" has never been standardized, but was just a common community convention—common enough it started to sneak into some bundle setups. All Node LTS versions and all bundlers I’m aware of now respect "exports", which means this is no longer needed.
"main" is still standard. And technically it’s superceded by exports, but we’ll just keep it around for safety. But "module" is safe to remove at this point.
| @@ -8,19 +8,12 @@ | |||
| }, | |||
| "license": "MIT", | |||
| "type": "module", | |||
| "main": "./dist/index.js", | |||
| "module": "./dist/index.js", | |||
| "types": "./dist/index.d.ts", | |||
There was a problem hiding this comment.
Same for "types"—at this point TypeScript can now just locate the types due to unbuild’s careful handling
| }, | ||
| "./*.js": "./*.mjs", |
There was a problem hiding this comment.
Unbuild intentionally ships .mjs so there’s no ambiguity about the .js files being ESM, and there’s no way to opt out of this behavior. While in our case we’ve been shipping ESM for a while, I still agree in general—.js files is a bit of a clusterfuck trying to sniff out package.jsons to know how to parse it.
This is encroaching on a breaking change for this library, but I’m just considering it a minor version because:
- Deep imports have never had a strict guarantee they’ll be preserved perfectly
- This extension rewriting should preserve
.jsimports as-authored - We’re still building files 1:1 to their original versions, just with
.mjsextensions, so even if a setup does break it’s a mild annoyance not a show-stopper
drwpow
force-pushed
the
unbuild
branch
3 times, most recently
from
cef463a to
5772987
Compare
| "test:js": "vitest run", | ||
| "test:ts": "tsc --noEmit", | ||
| "test:ts-no-strict": "tsc --noEmit -p test/no-strict-null-checks/tsconfig.json", | ||
| "test:exports": "pnpm run build && attw --pack .", |
There was a problem hiding this comment.
I really like the “are the types wrong” type tests, so added to all packages. We were also able to remove the cjs-resolves-to-esm rule 😎
| @@ -1,4 +1,5 @@ | |||
| .turbo | |||
| *.config.* | |||
There was a problem hiding this comment.
Ignore build.config.ts configs for unbuild in package publishing
| // Ship CommonJS-compatible bundle | ||
| emitCJS: true, | ||
| // Don’t bundle .js files together to more closely match old exports (can remove in next major) | ||
| output: { preserveModules: true }, |
There was a problem hiding this comment.
openapi-metadata was already using unbuild! But adding this config just shores it up more closely with the other packages without changing output too much.
|
Merging this not to bulldoze anyone; this is just a very unopinionated PR that is just package maintenance. The change does require a version-bump just to guard against accidental regressions, but anything that crops up is fixable/changeable. Overall this should be a net win for everybody, improving CJS support and build quality. |
Changes
Unbuild is a fantastic packaging system that is the best available in my opinion. Build all our packages with it to reduce maintenance and overhead, and improve support. It’s still ESM-first, so there should be no changes to runtime, client weight, or bundlesize downstream, and only improves stability and support.
This also aligns openapi-typescript, openapi-fetch, openapi-react-query, and swr-openapi to all be built the same way.
If this causes any issues, please file an issue! Anything that crops up is fixable with the Unbuild system because it’s based on Rollup.
How to Review
Checklist
docs/updated (if necessary)pnpm run update:examplesrun (only applicable for openapi-typescript)