feat(utoopack): support multiple server entries - #72
Open
fireairforce wants to merge 4 commits into
Open
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughUtoopack now supports named ChangesUtoopack server-entry support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant BuildPlan
participant UtoopackAdapter
participant ManifestGenerator
participant EmittedServerEntries
BuildPlan->>UtoopackAdapter: provide server-runtime and page-server entries
UtoopackAdapter->>UtoopackAdapter: resolve ordered named server entries
UtoopackAdapter->>ManifestGenerator: provide server entry configuration and stats
ManifestGenerator->>ManifestGenerator: select one JavaScript asset per entry
ManifestGenerator->>EmittedServerEntries: emit mapped entries and shared assets
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
fireairforce
marked this pull request as ready for review
August 3, 2026 07:57
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 478581979e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/bundler-utoopack/src/manifest-generator.ts (1)
326-338: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winThe single-asset short-circuit accepts an asset that does not belong to the entry.
If an entrypoint lists exactly one JavaScript asset,
selectServerJavaScriptAssetreturns it without any name check. If Utoopack emits only a shared chunk for that entrypoint, the generator maps the planned entry to the shared chunk and the manifest points renderers at the wrong file. Every other path in this function requires a name match.Check the name first, then fall back to the sole asset. The behavior stays the same for the common case and rejects an obvious mismatch.
♻️ Proposed refactor: prefer the named candidate
function selectServerJavaScriptAsset( entryName: string, assets: AssetGroup, ): string { - if (assets.js.length === 1) return assets.js[0] as string; const candidates = assets.js.filter((asset) => isNamedEntryAsset(entryName, asset), ); if (candidates.length === 1) return candidates[0] as string; + if (candidates.length === 0 && assets.js.length === 1) { + return assets.js[0] as string; + } throw new Error( `[evjs] Utoopack server stats entrypoint "${entryName}" must identify exactly one JavaScript entry asset; found ${assets.js.length} JavaScript assets and ${candidates.length} named candidates.`, ); }🤖 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 `@packages/bundler-utoopack/src/manifest-generator.ts` around lines 326 - 338, Update selectServerJavaScriptAsset to validate the asset name before accepting a single JavaScript asset: prefer the uniquely named candidate, and only fall back to the sole asset when it matches the entry name. Preserve the existing error behavior when neither condition is satisfied.packages/bundler-utoopack/tests/multi-server-entry.test.ts (1)
85-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
requireruns the built modules and each fixture callsconsole.logat import time.Every emitted entry logs during the test. Remove the
console.logcalls from the fixtures, or export a value and assert on it. The load check then stays meaningful without extra test output.♻️ Proposed refactor: export instead of log
fs.promises.writeFile( path.join(sourceDir, "server.ts"), - 'import { sharedPrimary } from "./shared-primary";\nconsole.log("server", sharedPrimary);\n', + 'import { sharedPrimary } from "./shared-primary";\nexport const serverEntry = sharedPrimary;\n', ),Apply the same change to
dashboard.server.tsanddetail.server.ts.🤖 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 `@packages/bundler-utoopack/tests/multi-server-entry.test.ts` around lines 85 - 99, Remove the import-time console.log calls from the dashboard.server.ts and detail.server.ts fixtures, replacing them with exported values if needed. Keep the require-based checks in the multi-server entry test unchanged so they verify each emitted entry loads without producing test output.
🤖 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 `@packages/bundler-utoopack/src/adapter/create-config.ts`:
- Around line 259-289: Snapshot the framework-owned server entry defensively
before configureBundler hooks run, preserving array contents rather than sharing
the same reference. Update createUtoopackConfig to pass this expectedServerEntry
snapshot to every assertUtoopackServerEntryMatchesPlan call, so in-place
push/splice mutations are detected while existing scalar and array comparisons
remain unchanged.
In `@packages/bundler-utoopack/tests/adapter.test.ts`:
- Around line 1042-1045: Update the vi.waitFor assertion around onBuildOutput
and onServerBundleReady to use an explicit timeout longer than the 500 ms stats
polling delay, ensuring the expectation remains reliable on slower environments.
In `@packages/bundler-utoopack/tests/multi-server-entry.test.ts`:
- Around line 76-84: Update the sharedAssets assertion in the multi-server entry
test to avoid matching the unstable or incorrect “server-shared” filename
pattern. Assert a stable invariant instead, preferably identifying an emitted
asset referenced by more than one server entrypoint, or use a documented
entry-independent asset name if one exists.
---
Nitpick comments:
In `@packages/bundler-utoopack/src/manifest-generator.ts`:
- Around line 326-338: Update selectServerJavaScriptAsset to validate the asset
name before accepting a single JavaScript asset: prefer the uniquely named
candidate, and only fall back to the sole asset when it matches the entry name.
Preserve the existing error behavior when neither condition is satisfied.
In `@packages/bundler-utoopack/tests/multi-server-entry.test.ts`:
- Around line 85-99: Remove the import-time console.log calls from the
dashboard.server.ts and detail.server.ts fixtures, replacing them with exported
values if needed. Keep the require-based checks in the multi-server entry test
unchanged so they verify each emitted entry loads without producing test output.
🪄 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 Plus
Run ID: b4052420-c62f-4f3a-8254-31c13ecd0ae1
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
packages/bundler-utoopack/package.jsonpackages/bundler-utoopack/src/adapter/create-config.tspackages/bundler-utoopack/src/adapter/index.tspackages/bundler-utoopack/src/manifest-generator.tspackages/bundler-utoopack/tests/adapter.test.tspackages/bundler-utoopack/tests/create-config.test.tspackages/bundler-utoopack/tests/manifest-generator.test.tspackages/bundler-utoopack/tests/multi-server-entry.test.ts
fireairforce
force-pushed
the
zoomdong/utoopack-multi-server-entry
branch
from
August 3, 2026 09:48
a0575ba to
c8f1ee6
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/bundler-utoopack/tests/multi-server-entry.test.ts (1)
70-106: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert selected entry assets and shared inventory in the integration test.
The test checks fact keys, shared files in stats, and module loadability. A regression that maps a page-server entry to
server.js, or removes a shared chunk fromfacts.emittedFiles.server, can still pass. Assert that each selected asset belongs only to its named stats entrypoint and that emitted server files containsharedAssets.Verify the entry-asset uniqueness invariant against the actual
@utoo/pack1.5.0 stats for this fixture.Proposed test update
- expect(Object.keys(facts.serverEntryAssets ?? {}).sort()).toEqual([ + const serverEntryAssets = facts.serverEntryAssets ?? {}; + expect(Object.keys(serverEntryAssets).sort()).toEqual([ "page-server-dashboard", "page-server-detail", "server", ]); @@ const sharedAssets = [...assetReferenceCounts] .filter(([, references]) => references > 1) .map(([asset]) => asset); expect(sharedAssets.length).toBeGreaterThan(0); + for (const [name, entry] of Object.entries(serverEntryAssets)) { + const [entryAsset] = entry.js; + if (!entryAsset) throw new Error(`Expected JavaScript asset for ${name}.`); + const statsEntrypoint = stats.entrypoints[name]; + if (!statsEntrypoint) throw new Error(`Expected stats entrypoint for ${name}.`); + expect( + statsEntrypoint.assets.map((asset) => + asset.name.replace(/^\.\//, ""), + ), + ).toContain(entryAsset); + expect(assetReferenceCounts.get(entryAsset)).toBe(1); + } + expect(facts.emittedFiles?.server).toEqual( + expect.arrayContaining(sharedAssets), + ); await expect( Promise.all( sharedAssets.map((asset) => @@ - for (const entry of Object.values(facts.serverEntryAssets ?? {})) { + for (const entry of Object.values(serverEntryAssets)) {🤖 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 `@packages/bundler-utoopack/tests/multi-server-entry.test.ts` around lines 70 - 106, Strengthen the integration assertions around facts.serverEntryAssets and stats.entrypoints: for each named server entry, assert its selected asset belongs to that entrypoint and is not associated with any other entrypoint, preserving the actual `@utoo/pack` 1.5.0 fixture behavior. Also assert every asset in sharedAssets is present in facts.emittedFiles.server, in addition to the existing filesystem and loadability checks.
🤖 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.
Nitpick comments:
In `@packages/bundler-utoopack/tests/multi-server-entry.test.ts`:
- Around line 70-106: Strengthen the integration assertions around
facts.serverEntryAssets and stats.entrypoints: for each named server entry,
assert its selected asset belongs to that entrypoint and is not associated with
any other entrypoint, preserving the actual `@utoo/pack` 1.5.0 fixture behavior.
Also assert every asset in sharedAssets is present in facts.emittedFiles.server,
in addition to the existing filesystem and loadability checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c4e05ac0-4e36-4559-8626-22e5a2b69351
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
packages/bundler-utoopack/package.jsonpackages/bundler-utoopack/src/adapter/create-config.tspackages/bundler-utoopack/src/adapter/index.tspackages/bundler-utoopack/src/manifest-generator.tspackages/bundler-utoopack/tests/adapter.test.tspackages/bundler-utoopack/tests/create-config.test.tspackages/bundler-utoopack/tests/manifest-generator.test.tspackages/bundler-utoopack/tests/multi-server-entry.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/bundler-utoopack/src/adapter/index.ts
- packages/bundler-utoopack/src/manifest-generator.ts
- packages/bundler-utoopack/package.json
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.
Summary
@utoo/packto1.5.0server-runtimeandpage-serverBuildPlan entries as named Utoopack server entriesserverEntryAssetswhile retaining shared server chunksRelated
Summary by CodeRabbit
New Features
Bug Fixes