chore: migrate build from tsup to tsdown and enable publint - #236
Merged
Conversation
Replace tsup with tsdown in all packages and validate the published output with publint on every build. Fixing the build surfaced a few packaging issues: - tsdown defaults `fixedExtension` to true when the platform is node, emitting `dist/index.mjs` and `dist/index.d.mts` while every main/module/types/exports field still pointed at `dist/index.js`. Set `fixedExtension: false` to keep the existing paths. - tsdown bundles type declarations through rolldown, which cannot inline CommonJS `.d.ts` files. Packages whose public API exposes framework types now declare those frameworks as peer dependencies so they stay external: express for the express adapter, fastify and @fastify/cookie for the fastify adapter. - The core package leaked express types through the default type parameters of `CompositeTokenSource`. Core is framework agnostic, so drop the defaults along with the `@types/express` dev dependency. Also declare `"sideEffects": false` and use the full git URL form for `repository.url`, both flagged by publint.
🦋 Changeset detectedLatest commit: 150672d The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 |
The Node 20 build failed because tsdown loads a TypeScript config through `unrun`, which is not installed; Node 22+ strips the types natively. Raise `engines.node` to >=22.0.0 across the workspace and replace Node 20 with Node 26 in the test matrix. tsdown picks the build target up from engines. Separately, the test runs failed on every Node version because the bump to cookie v2 renamed its exports: `parse` and `serialize` are now `parseCookie` and `stringifySetCookie`, with the latter taking a single object. This did not reproduce locally because the workspace still had cookie 1.0.2 linked. Only tests use those functions, so no published code is affected. Also move `cookie` in the express adapter to a dev dependency, as it is only used for a type in the tests.
Replace the `any` types flagged by `noExplicitAny` with concrete ones: - `JWTPayload` now declares the `tokens` and `isAuthenticated` fields the code actually reads, so the index signature can be `unknown`. - `TokenSigner.encryptObject` takes `Record<string, unknown>`, matching how the value is used. Callers passing an object are unaffected. - The fastify reply mock is typed instead of cast through `any`. Also drop the unused constructor argument from the cookie test adapter, flagged by `noUnusedPrivateClassMembers`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Finishes the in-progress tsup → tsdown migration so
pnpm buildpasses again, and wires uppublintto validate the published output on every build.Note: this branch is based on local
main, which was ahead oforigin/main, so it also carriesci: merge workflow filesandchore: mark graphql-yoga as peer dependency.Fixing the build
tsdown bundles type declarations through rolldown, which cannot inline CommonJS
.d.tsfiles. It externalizesdependenciesandpeerDependencies, but express and fastify were only devDependencies, so their types got pulled into the bundle and the build failed:expressas a peer dependency.fastifyand@fastify/cookieas peer dependencies.CompositeTokenSource. Core is framework agnostic (that is what the adapters are for), so the defaults are gone along with the now-unused@types/expressdev dependency.Enabling publint
Enabled through tsdown's built-in integration (
publint: true), so it runs as part ofpnpm buildand CI already covers it.It immediately caught a regression from the migration: tsdown defaults
fixedExtensionto true when the platform is node, so it was emittingdist/index.mjsanddist/index.d.mtswhile everymain/module/types/exportsfield still pointed atdist/index.js. Every package would have published broken. SettingfixedExtension: falsekeeps the existing paths.Two smaller things it flagged are fixed too:
"sideEffects": falseis now declared, andrepository.urluses the fullgit+https://….gitform.Also
tsdown.config.ts.linkedin the changesets config.Reviewer note
Removing the generic defaults on
CompositeTokenSourceis a breaking type change for anyone referencing the bare type in a type position (let s: CompositeTokenSource). Constructor calls likenew CompositeTokenSource([cookieSource])still infer both parameters and are unaffected. If that is too sharp an edge, the alternative is keeping defaults backed by a minimal locally-defined request/response shape.Verification
pnpm build(6/6, publint clean),pnpm test(80 tests),pnpm tsc(6/6) andpnpm lintall pass. The 12 lint warnings are pre-existing.