-
Notifications
You must be signed in to change notification settings - Fork 0
feat(tsconfig): add tsconfig.build.json for distribution #22
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
Conversation
This commit adds a `tsconfig.build.json` file to the project, which is a specialized TypeScript configuration for building the distribution package. The key changes include: - Setting `noEmit` to `false` to enable compilation - Enabling `declaration` and `declarationMap` to generate type definitions - Setting `sourceMap` to `true` to generate source maps - Configuring `outDir` and `rootDir` to output the compiled files to the `dist` directory - Excluding test files, distribution, and node_modules from the build
📝 WalkthroughSummary by CodeRabbit
WalkthroughIntroduces a GitHub Actions release workflow to publish to NPM, updates ESLint to ignore the dist output, restructures package.json for public distribution from dist with build/publish scripts, and adds a dedicated TypeScript build config (tsconfig.build.json) for emitting compiled JS and type declarations. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Maintainer
participant GitHub as GitHub Release Event
participant Workflow as Release Workflow
participant Job as release (ubuntu-latest)
participant NPM as npm Registry
Maintainer->>GitHub: Publish Release (tag)
GitHub-->>Workflow: Trigger on release: published
Workflow->>Job: Start job with permissions
rect rgb(245,245,255)
Job->>Job: Checkout repository
Job->>Job: Setup Bun (1.2.21)
Job->>Job: Set package.json version = github.ref_name
Job->>Job: bun install --frozen-lockfile
end
alt Prerelease
Job->>NPM: bun publish --tag canary
else Release
Job->>NPM: bun publish --tag latest
end
NPM-->>Job: Publish result
Job-->>Workflow: Complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 3
🧹 Nitpick comments (8)
eslint.config.ts (1)
8-10: Ignore pattern LGTM; consider a simpler glob
dist/**/*works, but**/dist/**is a bit more robust if any sub-packages or future folders appear.{ - ignores: ['dist/**/*'], + ignores: ['**/dist/**'], },tsconfig.build.json (2)
10-14: Use NodeNext + verbatimModuleSyntax for ESM packagesWith
"type": "module"and publishing ESM,"moduleResolution": "NodeNext"and"verbatimModuleSyntax": trueare the safer defaults. This avoids subtle interop issues and aligns TS emit with Node ESM.- "moduleResolution": "node", - "module": "ESNext", - "target": "ES2022", - "allowImportingTsExtensions": false, - "verbatimModuleSyntax": false + "moduleResolution": "NodeNext", + "module": "ESNext", + "target": "ES2022", + "allowImportingTsExtensions": false, + "verbatimModuleSyntax": true
7-9: Ship usable source maps by embedding sources (or disable maps)Since only
distis published, external consumers won’t havesrcby default. Either turn offsourceMapor embed sources into the maps.- "sourceMap": true, + "sourceMap": true, + "inlineSources": true,package.json (4)
3-13: “dev” isn’t a valid publishable versionYour workflow overwrites version at release time, but a manual publish would fail. Optional: switch to a prerelease-like placeholder (e.g.,
0.0.0-dev) to remain valid SemVer.- "version": "dev", + "version": "0.0.0-dev",
24-26: Align with source maps planIf you keep
sourceMap: true, embedding sources (tsconfig suggestion) avoids broken stack traces without shippingsrc/. Otherwise consider excluding*.mapfrom the published files."files": [ - "dist" + "dist" ], + "sideEffects": false,Note:
sideEffects: falseis optional but helps tree-shaking if applicable.
63-68: Make lint script target the workspace root explicitly
eslintwithout a path may lint nothing depending on the environment/ESLint version. Useeslint .. Also consider adding acleanscript.- "build": "tsc --project tsconfig.build.json && tsc-alias -p tsconfig.build.json", + "build": "tsc --project tsconfig.build.json && tsc-alias -p tsconfig.build.json", "format": "prettier --cache --write .", - "lint": "eslint", + "lint": "eslint .", "prepublishOnly": "bun run build", "typecheck": "tsc --noEmit"Additional (outside changed range):
+ "engines": { + "node": ">=18" + },
59-61: Is TypeScript truly a peer?If the runtime requires
typescript(e.g., codegen uses it), keeping as a peer is fine. If not, remove it frompeerDependencies. You can also mark it optional to reduce install friction."peerDependencies": { - "typescript": "~5.9.2" + "typescript": "~5.9.2" }, + "peerDependenciesMeta": { + "typescript": { "optional": true } + },.github/workflows/release.yml (1)
36-40: Optional: use npm CLI for provenance if Bun lacks itIf verification shows Bun doesn’t support provenance, switch to npm with OIDC.
- - name: Publish - env: - NPM_CONFIG_TOKEN: ${{ secrets.NPM_TOKEN }} - TAG: ${{ github.event.release.prerelease && 'canary' || 'latest' }} - run: bun publish --tag=${{ env.TAG }} + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version: '20' + registry-url: 'https://registry.npmjs.org' + + - name: Publish (npm with provenance) + env: + NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + TAG: ${{ github.event.release.prerelease && 'canary' || 'latest' }} + run: npm publish --provenance --tag "${TAG}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/release.yml(1 hunks)eslint.config.ts(1 hunks)package.json(2 hunks)tsconfig.build.json(1 hunks)
🔇 Additional comments (3)
tsconfig.build.json (1)
1-26: Path aliases are correctly configured
tsconfig.json defines"baseUrl": "."and"paths": { "@daxserver/validation-schema-codegen/*": ["./src/*"], … }, which tsconfig.build.json inherits—covering all non-relative imports under@daxserver/validation-schema-codegen/*. No changes needed.package.json (2)
15-23: Exports/entrypoints look good for ESM + typesMapping
main/moduletodist/index.jsand exposingtypesinexportsis consistent.
56-58: Good call adding tsc-alias; ensure it’s neededIf you rely on TS path aliases at runtime, keep it. If not, consider removing to simplify the toolchain.
| - name: Install Dependencies | ||
| run: bun install --frozen-lockfile | ||
|
|
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.
🛠️ Refactor suggestion
Build explicitly before publish (don’t rely on lifecycle)
To avoid CLI/lifecycle discrepancies, run the build step explicitly.
- name: Install Dependencies
run: bun install --frozen-lockfile
+ - name: Build dist
+ run: bun run build
+
- name: Publish📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install Dependencies | |
| run: bun install --frozen-lockfile | |
| - name: Install Dependencies | |
| run: bun install --frozen-lockfile | |
| - name: Build dist | |
| run: bun run build | |
| - name: Publish |
🤖 Prompt for AI Agents
.github/workflows/release.yml lines 33-35: the workflow currently only installs
dependencies and relies on package lifecycle to build before publishing; add an
explicit build step after installing (e.g., run: bun build or the project's
build command) so the artifact is produced deterministically before publish, and
ensure the build command runs before any publish/publish-related steps in the
job.
| "publishConfig": { | ||
| "access": "public", | ||
| "provenance": true, | ||
| "tag": "latest" | ||
| }, |
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.
💡 Verification agent
🧩 Analysis chain
Provenance + Bun publish: verify support
publishConfig.provenance: true requires provenance-aware publish. Please confirm bun publish supports npm provenance for your version; otherwise use npm publish --provenance.
🌐 Web query:
Does "bun publish" support npm provenance attestations equivalently to "npm publish --provenance" as of Bun 1.2.21?
💡 Result:
Short answer: No.
Evidence:
- Bun's publish docs (current CLI docs) don't mention a --provenance flag or NPM_CONFIG_PROVENANCE support for bun publish. [2]
- There is an open GitHub issue requesting implementation of a bun publish --provenance feature (issue #15601), indicating it was not implemented at the time of that issue. [1]
- npm documents how third‑party publishers can enable provenance (NPM_CONFIG_PROVENANCE, publishConfig, or calling npm publish --provenance). Bun's docs do not list those compatibility workarounds. [3]
If you want, I can check the exact bun 1.2.21 release notes or test a local bun 1.2.21 install to confirm behavior.
Remove publishConfig.provenance or switch to npm publish --provenance
Bun CLI (v1.2.21) does not support npm provenance attestations via bun publish (no --provenance flag) (bun.com, github.com) Use npm publish --provenance or drop the provenance field to enable attestations.
🤖 Prompt for AI Agents
In package.json around lines 28 to 32, the publishConfig currently includes
"provenance": true which Bun CLI does not support; either remove the
"provenance" property from publishConfig to allow bun publish to work, or keep
it but document and switch your publishing step to use npm (npm publish
--provenance) instead of bun so the provenance attestation can be produced;
update package.json accordingly (remove the field) or update CI/release scripts
to invoke npm publish --provenance and leave a comment in package.json
explaining the choice.
This commit adds a
tsconfig.build.jsonfile to the project, which is aspecialized TypeScript configuration for building the distribution package.
The key changes include:
noEmittofalseto enable compilationdeclarationanddeclarationMapto generate type definitionssourceMaptotrueto generate source mapsoutDirandrootDirto output the compiled files to thedistdirectory