build: fix release pipeline so first publish can ship#5
Conversation
`tsconfig.cjs.json` and `tsconfig.esm.json` were inheriting only the root `exclude` and so were compiling every `*.test.ts` into `dist/cjs/` and `dist/esm/`. The tarball then filtered them back out via `files`, but the build itself was doing wasted work and the glob list in `package.json` was carrying dead patterns (`!dist/testUtils`, `!dist/scratch.*`) that didn't match anything. Matched `todoist-sdk-typescript`'s pattern: exclude `**/*.test.ts` in both tsconfigs and keep the tarball filter to `!dist/**/*.test.js` + `!dist/**/*.test.d.ts`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
doistbot
left a comment
doistbot
left a comment
There was a problem hiding this comment.
This PR thoughtfully cleans up the TypeScript build configuration and package exclusions to prevent test files from being compiled into the release. These adjustments nicely refine the publishing pipeline to ensure a leaner final package. There are just a few minor suggestions noted for further refinement, including moving the duplicated exclusions into the base configuration, ensuring the test helpers in testUtils are also excluded, and introducing a packaging smoke test in CI to verify the tarball contents moving forward.
| @@ -1,5 +1,6 @@ | |||
| { | |||
| "extends": "./tsconfig.json", | |||
| "exclude": ["node_modules", "dist", "**/testData", "**/*.test.ts"], | |||
There was a problem hiding this comment.
[P2] Because src/testUtils contains .ts files that don't end in .test.ts, they will still be compiled into dist/cjs/testUtils (and esm/types). Consider adding "**/testUtils" to the exclude list here and in tsconfig.esm.json to prevent test helpers from being published to npm.
| @@ -1,5 +1,6 @@ | |||
| { | |||
| "extends": "./tsconfig.json", | |||
| "exclude": ["node_modules", "dist", "**/testData", "**/*.test.ts"], | |||
There was a problem hiding this comment.
[P2] This exclude array is now duplicated in both build tsconfigs and also repeats entries already defined in tsconfig.json. Since tsconfig.test.json already overrides exclude to [], you can move **/*.test.ts into the shared base tsconfig.json exclude list and drop these per-target copies to keep a single source of truth.
| "!dist/**/*.test.*", | ||
| "!dist/testUtils", | ||
| "!dist/scratch.*" | ||
| "!dist/**/*.test.js", |
There was a problem hiding this comment.
[P2] This changes the package contents to fix a release failure, but there’s still no regression test for the tarball contents. prepublishOnly builds and runs ATTW, yet nothing asserts that npm pack omits compiled *.test.* files, so a future tsconfig/files drift can silently reintroduce the same breakage. Add a small packaging smoke test in CI that inspects npm pack --json output or the packed file list.
2 participants
Context
ReleaseandDeploy Docs to Pagesare both failing onmain. Three issues; this PR handles one, the others are noted below.What was changed
tsconfig.cjs.json/tsconfig.esm.jsonnow exclude**/*.test.ts. The build was compiling tests intodist/only forfilesto filter them back out.package.jsonfilescleaned up to matchtodoist-sdk-typescript: drop dead!dist/testUtils/!dist/scratch.*, keep!dist/**/*.test.js+!dist/**/*.test.d.ts.Done outside this PR
build_type=workflow, matchestodoist-sdk-typescript). FailedDeploy Docs to Pagesrerun now passes.Still to do
ReleasefailsEINVALIDNPMTOKENbecause@doist/comms-sdkdoesn't exist on npm yet — trusted publishing can't bootstrap a new package. Fix:npm publishonce locally, then enable Trusted Publisher on npmjs.com for this repo'srelease.yml. After that the existing tokenless workflow works the same way@doist/todoist-sdkdoes.