Skip to content

Fix: derive the bundled outfile from the entry in integrations/build.js - #784

Open
AmaadMartin wants to merge 3 commits into
mainfrom
fix/integrations-bundled-outfile-tracks-entry
Open

Fix: derive the bundled outfile from the entry in integrations/build.js#784
AmaadMartin wants to merge 3 commits into
mainfrom
fix/integrations-bundled-outfile-tracks-entry

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 7, 2026

Copy link
Copy Markdown
Owner

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):

N/A

  1. Or, if no issue exists, describe the change:

Problem: integrations/build.js hardcoded the bundled output filename to index.js, ignoring the entry it was given. The web target therefore compiled src/index_web.ts into dist/web/index.js, and dist/web/index_web.js was never emitted. That is the path integrations/package.json advertises as "browser", so after npm run build:bundle the field dangles. The unbundled build is unaffected because it globs ./src/**/*.ts into outdir and ignores entry.

Solution: Drop outfile and give both modes the same outdir. esbuild then derives each output filename from its entry, which is the naming the bundled branch was doing by hand and getting wrong. Both fields are unconditional once the branch is gone, so they move into the buildOptions literal:

const buildOptions = {
  entryPoints: bundle ? [`./src/${entry}`] : ['./src/**/*.ts'],
  outdir: `./dist/${targetDir}`,
  target: platformBuildTargets[platform],
  // ...unchanged
};

Post-construction mutation is left to banner, the only field that is still conditional.

esm and cjs are unchanged because their entry is already index.ts, so main and module keep resolving. I diffed the whole of dist/ before and after: both the bundled and the unbundled output are byte-identical to an outfile-based fix, and the unbundled output is byte-identical to main.

Measured on this branch:

# before
$ rm -rf integrations/dist && npm run build:bundle --workspace integrations
  dist/esm/index.js  369b
  dist/cjs/index.js  735b
  dist/web/index.js  369b       <-- built from src/index_web.ts
$ ls integrations/dist/web/index_web.js
ls: cannot access '.../dist/web/index_web.js': No such file or directory

# after
$ rm -rf integrations/dist && npm run build:bundle --workspace integrations
  dist/esm/index.js      369b
  dist/cjs/index.js      735b
  dist/web/index_web.js  373b

core/build.js has the identical defect and is deliberately out of scope. core/build.js:65-71 hardcodes the same outfile, core/build.js:109 passes entry: 'index_web.ts', and core/package.json declares the same "browser": "./dist/web/index_web.js" — so npm run build:bundle --workspace core breaks that field too.

I am not fixing it here because google#614 is open and already fixes it, with the same outfile hunk plus a rewrite of the rest of that function: new browser targets, packages: 'browser' ? 'bundle' : 'external', an alias block of browser shims, format === 'esm' && platform === 'node' banner gating, and a web target forced to bundle: true. Editing core/build.js on this branch would collide with a live review for no gain. When #614 lands, core/build.js can take this same outdir collapse as a follow-up; that is a strictly smaller change than the one #614 is already carrying.

Collision check, per the contribution workflow: five open PRs also edit integrations/build.js (#430, #552, #553, #555, #670). All five change the license banner and the createRequire preamble. None touches the outfile assignment, so this branches from main rather than stacking.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.

Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.

New file tests/integration/build_setup/integrations_build_test.ts runs both build scripts and asserts that the files named by browser, main and module all exist and are non-empty. It reads those paths from integrations/package.json instead of hardcoding them, so it checks the actual manifest contract.

The bundled case also asserts that the browser target holds exactly one .js file. Sharing one outdir between the two modes means the entry list is now the only difference between them, so without that assertion the two cases assert the same thing and a build:bundle that quietly stopped bundling would still pass. Mutation 2 below is that regression.

It had to go under tests/integration/ rather than integrations/test/: npm run test:coverage, the command CI runs, does not run the unit:integrations vitest project.

Each case deletes integrations/dist before it builds. Without that both cases pass on the unfixed code, because CI's earlier npm run build step leaves a valid dist/web/index_web.js on disk. The bundled case runs first, so the tree is left holding the plain build's output. Nothing else in the repo reads integrations/dist — every vitest project aliases @google/adk-integrations to integrations/src.

Mutation checks. Four mutations were run against the final code and every one failed.

  1. integrations/build.js exactly as on main:
 × emits every entry point the manifest names, bundled
 AssertionError: promise rejected "Error: ENOENT: no such file or directory,… { …(4) }" instead of resolving
 Caused by: Error: ENOENT: no such file or directory, access '.../integrations/dist/web/index_web.js'
 Tests  1 failed | 1 passed (2)

The unbundled case still passes, which is the right split: the test pins the bundled path specifically, not merely that a build ran.

  1. Swap the ternary arms, so build:bundle globs every source instead of bundling the entry:
 × emits every entry point the manifest names, bundled
 AssertionError: the bundled build must emit one bundle, not one file per source:
   expected [ 'index.js', 'index_web.js', …(1) ] to deeply equal [ 'index_web.js' ]
  1. Drop ${targetDir} from outdir, which proves both cases carry signal:
 × emits every entry point the manifest names, bundled
 × emits every entry point the manifest names, unbundled
 Tests  2 failed (2)
  1. Hardcode the bundled entry to index.ts, which is the defect restated at the entry rather than the outfile:
 × emits every entry point the manifest names, bundled
 Caused by: Error: ENOENT: no such file or directory, access '.../integrations/dist/web/index_web.js'

Manual End-to-End (E2E) Tests:

Please provide instructions on how to manually test your changes, including any necessary setup or configuration.

npm install
npm run build
rm -rf integrations/dist && npm run build:bundle --workspace integrations
ls integrations/dist/web/index_web.js     # exists after the fix, absent before

Local validation on the pushed commit:

npx vitest run --project integration \
  tests/integration/build_setup/integrations_build_test.ts   2 passed (3.8s)
npm run build                                                exit 0
npm run lint                                                 exit 0
npm run format:check                                         exit 0
bash scripts/check_license.sh                                exit 0
npx tsc --noEmit                                             0 errors in the changed files

npx tsc --noEmit reports pre-existing errors elsewhere under core/test, unrelated to this change and not run by CI.

CI is green on all three legs of the validation matrix, plus check-license and the cross-language job. The new test ran and passed on each leg. It takes 2.8s on windows-latest, the leg at risk, because it shells out to npm run ... --workspace integrations.

Earlier attempts on this same commit were red on three pre-existing timeout flakes this change does not touch: tests/integration/app_loader/app_loader_test.ts, core/test/code_executors/unsafe_local_code_executor_test.ts and tests/integration/build_setup/build_setup_test.ts.

They are not mine. Across the last 20 failed validation runs on this fork, app_loader_test.ts times out on 13 branches, including main itself and the ci-control-main-baseline branch. The other two time out on four more branches that do not carry this change. Two further observations rule out an indirect effect:

  • On the windows run, unsafe_local_code_executor_test.ts timed out at 18:47:25 and the new build test ran at 18:51:45. The failure preceded my test by four minutes, so the builds it shells out to were not starving it.
  • The new test deletes integrations/dist, but nothing in the suite reads it. Every vitest project aliases @google/adk-integrations to integrations/src, and the build_setup fixtures depend only on core and dev, neither of which depends on integrations.

I am not fixing these flakes here, because they are unrelated breakage.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.

The bundled branch hardcoded the output filename to index.js, so the web
target compiled src/index_web.ts into dist/web/index.js. The path
integrations/package.json advertises as "browser", dist/web/index_web.js,
was never emitted by npm run build:bundle.

Derive the filename from the entry instead. The esm and cjs targets are
unchanged because their entry is already index.ts.
Amaad Martin added 2 commits August 7, 2026 11:20
esbuild derives each output filename from its entry when outdir is set,
so the bundled branch does not need to name the file itself. Giving both
modes the same outdir collapses the branch to two lines and removes the
hand-rolled extension swap.

Output is byte-identical to the outfile-based fix in both modes, and the
unbundled output is byte-identical to main.

The bundled test case now also asserts the browser target holds exactly
one .js file. The entry list is the only remaining difference between the
two modes, so without it a build:bundle that stopped bundling would pass.
Both fields are unconditional once the bundle branch is gone, so assigning
them after construction buys nothing and separates the two banner
conditionals. Post-construction mutation is now reserved for banner, which
is the only genuinely conditional field.

The emitted dist/ is byte-identical in both modes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant