fix(dal-runtime): package codegen templates#6030
Conversation
📝 WalkthroughWalkthroughThe dal-runtime build now copies ChangesTemplate Asset Build
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request configures templates as build assets in tsdown.config.ts by copying them from src/templates to dist/templates. It also adds a corresponding unit test in CodeGenerator.test.ts to verify this configuration. I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #6030 +/- ##
==========================================
+ Coverage 81.98% 82.17% +0.18%
==========================================
Files 678 678
Lines 20838 20842 +4
Branches 4154 4155 +1
==========================================
+ Hits 17085 17127 +42
+ Misses 3235 3207 -28
+ Partials 518 508 -10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes @eggjs/dal-runtime packaging so its Nunjucks templates are included in published artifacts, preventing “template not found” failures in clean consumer installs.
Changes:
- Update
tsdownbuild config to copysrc/templatesintodist/templates. - Add a unit test that asserts the build config includes the expected template asset copy rule.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tegg/core/dal-runtime/tsdown.config.ts | Copies Nunjucks templates into dist/ so they’re present in the published package. |
| tegg/core/dal-runtime/test/CodeGenerator.test.ts | Adds a test to lock in the tsdown asset-copy configuration for templates. |
| it('should configure templates as build assets', async () => { | ||
| const config = (await import('../tsdown.config.ts')).default as any; | ||
| assert.deepEqual(config.copy, [ | ||
| { | ||
| from: 'src/templates', | ||
| to: 'dist/templates', | ||
| }, | ||
| ]); | ||
| }); |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tegg/core/dal-runtime/test/CodeGenerator.test.ts (1)
18-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftVerify the emitted artifact, not only the config object.
This test can pass even when the build places files under the wrong nested directory. Add an integration assertion that runs the package build in a temporary output directory and verifies a representative template exists at
dist/templates/....🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tegg/core/dal-runtime/test/CodeGenerator.test.ts` around lines 18 - 26, Extend the test “should configure templates as build assets” to run the package build with a temporary output directory, then assert that a representative file from src/templates is emitted under dist/templates/... . Keep the existing config.copy assertion, and clean up the temporary build output after the integration check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tegg/core/dal-runtime/tsdown.config.ts`:
- Around line 7-12: Update the copy configuration so the `src/templates`
contents are placed directly under the output directory’s `templates` path; in
the `copy` entry, omit `to` or set it to `dist` rather than `dist/templates`,
ensuring `CodeGenerator` can find templates at `dist/templates`.
---
Nitpick comments:
In `@tegg/core/dal-runtime/test/CodeGenerator.test.ts`:
- Around line 18-26: Extend the test “should configure templates as build
assets” to run the package build with a temporary output directory, then assert
that a representative file from src/templates is emitted under
dist/templates/... . Keep the existing config.copy assertion, and clean up the
temporary build output after the integration check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 35180647-b249-4d44-8dd5-71e588402887
📒 Files selected for processing (2)
tegg/core/dal-runtime/test/CodeGenerator.test.tstegg/core/dal-runtime/tsdown.config.ts
| copy: [ | ||
| { | ||
| from: 'src/templates', | ||
| to: 'dist/templates', | ||
| }, | ||
| ], |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n '"tsdown"|"outDir"|from:|to:' \
--glob 'package.json' --glob '*lock*' --glob 'tsdown.config.ts' .Repository: eggjs/egg
Length of output: 1908
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant config and surrounding package settings.
cat -n tegg/core/dal-runtime/tsdown.config.ts
printf '\n--- package.json ---\n'
cat -n tegg/core/dal-runtime/package.json 2>/dev/null || true
printf '\n--- root package.json ---\n'
cat -n package.json | sed -n '1,120p'
# Look for repo-local tsdown copy-path examples and any notes about outDir/copy behavior.
printf '\n--- tsdown configs in repo ---\n'
rg -n "copy:\s*\[|from:\s*'src/templates'|to:\s*'dist/templates'|outDir" \
tegg tools packages examples --glob 'tsdown.config.ts' --glob 'package.json'Repository: eggjs/egg
Length of output: 7495
🌐 Web query:
tsdown copy entry to destination basename appends source basename docs Interface.CopyEntry
💡 Result:
In tsdown, the CopyEntry interface is used to define rules for copying files or directories from a source location to a destination during the build process [1][2][3]. The behavior regarding source and destination basenames depends on how the entry is configured [2][3]: 1. Default Behavior: When providing a string for a copy entry, tsdown defaults to placing the file or directory into the output directory (outDir) using its original basename [4]. 2. CopyEntry Object: By using an object with the CopyEntry interface, you can explicitly define the destination path using the to property [2][3]. - If the flatten option is set to true (which is the default), files are copied directly into the destination folder, effectively stripping their original directory structure and using the source basename as the file name [2][3]. - If you need to transform the destination name, the rename property within CopyEntry allows you to provide a string or a function to modify the destination file or folder name [2][3]. The internal implementation uses the resolveCopyEntry function, which resolves the destination path by joining the destination folder with the base name (or the renamed target) of the source file [2][3]. For full technical details, you can refer to the official API documentation for Interface: CopyEntry [1].
Citations:
- 1: https://main.tsdown.dev/reference/api/interface.copyentry
- 2: https://github.com/rolldown/tsdown/blob/42fe72e0a26f46ceb834bcccfad8520e852a3b67/src/features/copy.ts
- 3: https://github.com/rolldown/tsdown/blob/d424e0890d75245a8e8b8e067f8d15d8100a34eb/src/features/copy.ts
- 4: rolldown/tsdown@512ea97
Use the output directory as the copy destination base. from: 'src/templates' with to: 'dist/templates' resolves to dist/templates/templates, so CodeGenerator won’t find templates at dist/templates in a clean install. Omit to or set it to dist.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tegg/core/dal-runtime/tsdown.config.ts` around lines 7 - 12, Update the copy
configuration so the `src/templates` contents are placed directly under the
output directory’s `templates` path; in the `copy` entry, omit `to` or set it to
`dist` rather than `dist/templates`, ensuring `CodeGenerator` can find templates
at `dist/templates`.
|
Superseded by #6033. This patch is already included in the beta.24 baseline branch (codex/release-beta24-patchless) and was published with v4.1.2-beta.24. |
What changed
Copy
src/templatesintodist/templatesthrough the package tsdown configuration.Why
@eggjs/dal-runtimepublishes onlydist, but its Nunjucks templates were not copied by the build. A clean consumer install therefore fails DAL generation withtemplate not foundand currently requires Chair-bin to copy three files into node_modules during postinstall.Verification
vitest run --project @eggjs/dal-runtime tegg/core/dal-runtime/test/CodeGenerator.test.tsSummary by CodeRabbit
Bug Fixes
Tests