Skip to content

Chore: remove the dead dev/src/browser ignore entries from eslint.config.js and .prettierignore - #454

Open
AmaadMartin wants to merge 1 commit into
mainfrom
fix/remove-dead-dev-src-browser-ignores
Open

Chore: remove the dead dev/src/browser ignore entries from eslint.config.js and .prettierignore#454
AmaadMartin wants to merge 1 commit into
mainfrom
fix/remove-dead-dev-src-browser-ignores

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Aug 1, 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):
    Closes: #issue_number
    Related: #issue_number
  2. Or, if no issue exists, describe the change:
    Problem: eslint.config.js and .prettierignore both carve out dev/src/browser, a path that no longer exists anywhere in the repository.

The vendored ADK Web bundle lived at dev/src/browser/ (minified chunk-*.js, main-*.js, styles-*.css, index.html, assets/) until google#427 ("feat: download ADK Web assets dynamically and serve from shared folder", commit b8ee5d81) switched it to a build-time download. That PR deleted the vendored files but left both ignore entries behind, so the two configs have referenced a nonexistent path ever since.

A stale carve-out is worse than no carve-out: every future reader auditing "what does tooling skip?" has to resolve the path and discover it is a ghost. That cost was paid recently while auditing which files the wildcard coverage include glob dev/src/**/*.ts (vitest.config.ts:126) sweeps into the coverage denominator.

Solution: Delete the two dead entries. Exactly two lines change:

 .prettierignore  | 1 -
 eslint.config.js | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)
  • eslint.config.js:14ignores: ["**/dist/**", "dev/src/browser/**"]ignores: ["**/dist/**"]
  • .prettierignore:3dev/src/browser deleted

No replacement ignore was added, on purpose. The browser assets are now downloaded by ensureBrowserAssets() (dev/build.js:89-132) into dev/dist/browser (dev/build.js:90), which is already covered by the surviving "**/dist/**" (ESLint) and dist/ (Prettier, gitignore syntax, matches at any depth) entries, and is untracked via .gitignore:1. Adding dev/dist/browser or **/browser/** would recreate exactly the redundancy this change removes.

scripts/check_license.sh was deliberately left alone, including its browser prune (scripts/check_license.sh:7-8). Whether that prune is itself redundant is a separate question and is out of scope here.

Collision check. Before writing anything I listed all 300 open PRs on the fork and diffed the six that touch eslint.config.js (#435, #426, #422, #346, #332, #325). None of them removes the dead entry — five of them re-emit the same line with single quotes as a side effect of the repo's lint-staged Prettier hook (- ignores: ["**/dist/**", "dev/src/browser/**"] / + ignores: ['**/dist/**', 'dev/src/browser/**']), and #332 does not touch that line at all. No PR overlaps .prettierignore. This PR therefore branches from main rather than stacking; the textual overlap on line 14 is a one-line conflict resolvable in either merge order.

Pre-commit hook bypassed for this one commit, disclosed deliberately. lint-staged runs prettier --write over **/*.{js,ts}, and eslint.config.js is not Prettier-formatted on main (npx prettier --check eslint.config.js warns on it today — the root format:check script only globs **/*.ts, so CI has never checked it). Committing through the hook would reflow the whole file — single quotes, reordered imports, trailing commas, ~17 lines — and bury a two-line deletion in unrelated churn. The hook's actual checks were run by hand instead and are clean: npx secretlint "**/*" (exit 0) and eslint eslint.config.js (exit 0). The hook path itself is verified working, see the E2E section below.

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. — No tests added, intentionally. This change adds zero lines of executable code; it deletes one array element from a lint config and one line from an ignore file, so there is no new code to cover. A meta-test walking the ignore files and asserting each pattern resolves to a real path would immediately fail on the dist patterns, which legitimately point at generated directories that do not exist in a clean checkout. Nothing in the repo reads either file programmatically — grep -rn "prettierignore\|eslint\.config" --include='*.{ts,js,json,yaml,yml}' (excluding node_modules, dist, the lockfile) returns a single self-reference inside eslint.config.js itself — so no test can observe this change.
[x] All unit tests pass locally. — The full suite was not run: it is unaffected by construction (vitest.config.ts never reads either file, and no .ts file is added or removed, so the coverage denominator and every threshold in vitest.config.ts:121-137 are untouched). CI runs npm run test:coverage on this PR and gates it.

Direct before/after evidence that no file changed lint/format status (npm install completed first):

check before after
npx eslint "**/*.ts" -f json — files reported 531 531
identical file set (sorted filePath compare) true
identical results (filePath/errorCount/warningCount) true
npm run format:check All matched files use Prettier code style! (exit 0) byte-identical output, empty diff (exit 0)

Proof the removed entries were live config, not a no-op edit — the same probe before and after the change:

path ESLint isPathIgnored before → after Prettier --file-info before → after
dev/src/browser/main.js (the ghost path) truefalse truefalse
dev/dist/browser/main.js (real build output) truetrue truetrue

The first row is the mutation-style proof: both tools really did have a rule for that path and no longer do. It is behaviourally inert only because no such path exists — find . -type d -name browser -not -path './node_modules/*' returns nothing, and find dev/src -type d lists only cli, cli/deploy, conformance, integration, server, utils. The second row is the safety property: the surviving dist patterns still cover the downloaded assets.

Dead-reference checkgrep -rn "dev/src/browser" . --exclude-dir=node_modules --exclude-dir=.git now returns no matches (exit 1).

Full CI gate run locally, in the order .github/workflows/validation.yaml runs it:

step result
npm install ok
npx secretlint "**/*" exit 0
npm run build exit 0 — network available, adk-web-browser.zip downloaded and extracted to dev/dist/browser
npm run test:coverage not run locally (see above); gated by CI
npm run lint exit 0
npm run format:check exit 0
npm run docs:check exit 0

npm run ts:check is not a CI step and is red on main for an unrelated pre-existing reason (Cannot find module '@google/adk' in core/test/** when workspace dist output is absent). It is unaffected by a lint-config edit.

Build output still lands and is still ignored (after a real npm run build, so these are the actual downloaded files, not hypotheticals):

  • ls dev/dist/browseradk_favicon.svg, assets/, chunk-27SWUPRL.js, … index.html
  • git status --porcelain dev/dist → empty (still gitignored)
  • ESLint.isPathIgnored('dev/dist/browser/chunk-27SWUPRL.js')true; same for index.html
  • npx prettier --file-info dev/dist/browser/index.html"ignored": true
  • npx eslint "**/*.ts" still reports 531 files with dist/ populated, i.e. "**/dist/**" is still excluding every generated .d.ts

Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.

  1. lint-staged pre-commit path still works. On a scratch branch, with the modified configs in place, stage and commit an edit to a dev/src/**/*.ts file:
    git checkout -b scratch/hook-probe
    printf "\nexport const hookProbe = 'probe';\n" >> dev/src/utils/logger.ts
    git add dev/src/utils/logger.ts && git commit -m "scratch: hook probe"
    git checkout - && git branch -D scratch/hook-probe
    huskylint-staged ran secretlint, eslint --fix and prettier --write over the staged file and the commit succeeded. A first attempt that appended only a trailing newline was correctly rejected by lint-staged with "prevented an empty git commit" after prettier --write normalised it away — i.e. the formatter is genuinely executing on dev/src TypeScript. Scratch branch discarded.
  2. Read both configs end to end and confirm every surviving pattern maps to something real: **/dist/** and dist/ (generated build output, .gitignore:1) and node_modules/. .prettierignore is now exactly dist/\nnode_modules/\n. No entry points at a nonexistent source path.

CI result on this PR: all checks green — run-tests (ubuntu-latest), run-tests (macos-latest), run-tests (windows-latest), run-tests, check-license, auto-assign. The first windows-latest attempt failed in tests/integration/a2a/basic/a2a_agent_test.ts with Error: CLI exited prematurely with code 1 (tests/integration/test_case_utils.ts:341), a spawned-CLI integration flake that also fails on this fork's main (run 30669370416, same job, tests/integration/adk_web/webui_test.ts, same "1 failed | 223 passed | 20 skipped" shape). It is unrelated to this diff — nothing in the test suite reads either config file — and the job passed on re-run.

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. — n/a, two deletions.
[x] I have added tests that prove my fix is effective or that my feature works. — no tests are possible or warranted here; the before/after ignore probe above is the equivalent evidence.
[x] New and existing unit tests pass locally with my changes.

The vendored ADK Web bundle lived at dev/src/browser/ until google#427
switched it to a build-time download into dev/dist/browser. That PR deleted the
vendored files but left both ignore entries behind, so eslint.config.js and
.prettierignore have pointed at a nonexistent path ever since.

Remove them. No replacement pattern is added: "**/dist/**" (ESLint) and dist/
(Prettier) already cover the downloaded assets, and dist/ is gitignored.

The pre-commit hook is bypassed for this commit only: lint-staged runs
'prettier --write' over **/*.{js,ts}, and eslint.config.js is not
Prettier-formatted on main, so the hook would reflow the whole file and bury a
two-line deletion in unrelated churn. 'eslint eslint.config.js' and
'npx secretlint "**/*"' were both run manually and are clean.
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