fix(build): make the web build loadable outside Node - #837
Open
niravanu wants to merge 1 commit into
Open
Conversation
`dist/web` could only be loaded by Node, so `@google/adk` could not be
bundled for a browser, a worker, or any edge runtime. Two independent
causes, both in `core/build.js`.
**The createRequire banner was applied to the browser build.** The shim
exists so an ESM build can reach a CommonJS dependency, which is a Node
concern, but the condition was `format === 'esm'` — and the web build is
browser + esm. Every one of the 256 files in `dist/web` therefore opened
with `import {createRequire} from 'module'`, including leaf modules with
no Node dependency at all. Bundlers targeting a browser resolve that as
a missing package and stop.
**The browser target included Safari 11, which predates async
generators.** esbuild lowered them, and its lowering of
`yield* super.method()` emits `__yieldStar(super.method())` in a scope
where `super` is a syntax error, so `models/apigee_llm.js` did not parse.
Safari 11 was never really supported — the library uses async generators
throughout, and they are ES2018. The target only claimed it.
Measured before and after with wrangler, since Cloudflare Workers is the
case that fails hardest:
before ✘ [ERROR] Unexpected "super"
dist/web/models/apigee_llm.js:142:25
after Total Upload: 4652.19 KiB / gzip: 795.49 KiB
And it runs, not merely builds: a Workflow with two FunctionNodes joining
into a JoinNode, plus a RequestInput, constructed inside workerd and
answering 200.
Adds a regression test. It asserts the two specific failures — no
`createRequire` anywhere in `dist/web`, and every file parses — and
deliberately does not assert that nothing imports a Node builtin, because
several modules legitimately do (`file_artifact_service`,
`unsafe_local_code_executor`, `local_environment`, `skills/loader`,
`a2a/*`) and are only reached by importing them. The banner was different
in kind: it was on everything. The test skips when `dist/web` has not
been built, and it fails on the previous build and passes on this one.
The rest of the suite imports `src`, which is why neither of these was
visible to it.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
kalenkevich
approved these changes
Aug 31, 2026
kalenkevich
left a comment
Collaborator
There was a problem hiding this comment.
Totally make sense, thanks!
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
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.
dist/webcould only be loaded by Node, so@google/adkcould not be bundled for a browser, a worker, or any edge runtime. Two independent causes, both incore/build.js.The createRequire banner was applied to the browser build. The shim exists so an ESM build can reach a CommonJS dependency, which is a Node concern, but the condition was
format === 'esm'— and the web build is browser + esm. Every one of the 256 files indist/webtherefore opened withimport {createRequire} from 'module', including leaf modules with no Node dependency at all. Bundlers targeting a browser resolve that as a missing package and stop.The browser target included Safari 11, which predates async generators. esbuild lowered them, and its lowering of
yield* super.method()emits__yieldStar(super.method())in a scope wheresuperis a syntax error, somodels/apigee_llm.jsdid not parse. Safari 11 was never really supported — the library uses async generators throughout, and they are ES2018. The target only claimed it.Measured before and after with wrangler, since Cloudflare Workers is the case that fails hardest:
before ✘ [ERROR] Unexpected "super"
dist/web/models/apigee_llm.js:142:25
after Total Upload: 4652.19 KiB / gzip: 795.49 KiB
And it runs, not merely builds: a Workflow with two FunctionNodes joining into a JoinNode, plus a RequestInput, constructed inside workerd and answering 200.
Adds a regression test. It asserts the two specific failures — no
createRequireanywhere indist/web, and every file parses — and deliberately does not assert that nothing imports a Node builtin, because several modules legitimately do (file_artifact_service,unsafe_local_code_executor,local_environment,skills/loader,a2a/*) and are only reached by importing them. The banner was different in kind: it was on everything. The test skips whendist/webhas not been built, and it fails on the previous build and passes on this one.The rest of the suite imports
src, which is why neither of these was visible to it.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):
2. Or, if no issue exists, describe the change:
If applicable, please follow the issue templates to provide as much detail as
possible.
Problem:
A clear and concise description of what the problem is.
Solution:
A clear and concise description of what you want to happen and why you choose
this solution.
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:
Please include a summary of passed npm test results.
Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any
necessary setup or configuration. Please provide logs or screenshots to help
reviewers better understand the fix.
Checklist
Additional context
Add any other context or screenshots about the feature request here.