feat(ts_test): forward untyped_packages to the generated compiles - #158
Open
mikn wants to merge 1 commit into
Open
feat(ts_test): forward untyped_packages to the generated compiles#158mikn wants to merge 1 commit into
mikn wants to merge 1 commit into
Conversation
ts_compile's `untyped_packages` keeps a global-script package out of one target's type program, and the leak it exists for arrives through a dep's npm closure: the tsconfig writer offers every package in every first-party dep's transitive npm closure a `paths` key, so a `.d.ts` import of a package only a devDependency declared resolves in the sandbox where pnpm leaves it unresolved, and the global script it names merges into the program ahead of the `types` entries. A ts_test's program is written from the same deps by the same writer, and the macro had no `untyped_packages` parameter, so a test with the leak had no way to say it. `ts_test` takes `untyped_packages = None` and forwards it to every ts_compile it generates: the one over `srcs`, and the ones _compile_setup_sources makes over the TypeScript entries of `setup_files` and `global_setup`, which compile with the same deps. //tests/untyped_packages gains the pair on the program ts_test generates: `leaks_test` (manual) is leaks.ts as a vitest file on the same wrangler dep, `excluded_test` is the same file with the attribute and a setup file carrying the same call and `import()`, compiled under it, and three analysis tests read the generated tsconfigs. Red on 2bcc23f with the manual half in the tree (rt-evidence/logs/fix8r/P/01-red-leaks-test-compile.log): $ bazelisk --output_base=/var/tmp/fix8-P-ob build --noshow_progress //tests/untyped_packages:_leaks_test_compile [... 4 lines ...] INFO: Analyzed target //tests/untyped_packages:_leaks_test_compile (478 packages loaded, 25370 targets configured, 125 aspect applications). ERROR: /home/mikn/devel/lovable/rt-fix-p/tests/untyped_packages/BUILD.bazel:253:8: TsgoDeclare //tests/untyped_packages:_leaks_test_compile failed: (Exit 1): tsc failed: error executing TsgoDeclare command (from ts_compile rule target //tests/untyped_packages:_leaks_test_compile) external/+ts+tsgo_linux_amd64/lib/tsc --project bazel-out/k8-fastbuild/bin/tests/untyped_packages/_leaks_test_compile.tsconfig.json [... 2 lines ...] tests/untyped_packages/leaks.test.ts(6,15): error TS2769: No overload matches this call. The last overload gave the following error. Argument of type 'HTMLDivElement' is not assignable to parameter of type 'string | ReadableStream<any> | Response'. [... 3 lines ...] ERROR: Build did NOT complete successfully [... 1 lines ...] # exit=1 and with the attribute written on a ts_test (03-red-package.log): $ bazelisk --output_base=/var/tmp/fix8-P-ob test --noshow_progress --nocache_test_results --local_test_jobs=2 --test_summary=short //tests/untyped_packages/... ERROR: Traceback (most recent call last): File "/home/mikn/devel/lovable/rt-fix-p/tests/untyped_packages/BUILD.bazel", line 270, column 8, in <toplevel> ts_test( File "/home/mikn/devel/lovable/rt-fix-p/ts/private/ts_test.bzl", line 1082, column 5, in ts_test def ts_test( Error: ts_test() got unexpected keyword argument: untyped_packages [... 7 lines ...] ERROR: Skipping '//tests/untyped_packages/...': Error evaluating '//tests/untyped_packages/...': error loading package 'tests/untyped_packages': Package 'tests/untyped_packages' contains errors [... 4 lines ...] ERROR: Couldn't start the build. Unable to run tests [... 1 lines ...] # exit=1 The setup compile is an assertion only because the setup file loads wrangler's declaration entry too: with the attribute stripped from `excluded_test`, the DOM call alone compiles (14-control-setup-compile-no-attr.log) $ bazelisk --output_base=/var/tmp/fix8-P-ob build --noshow_progress //tests/untyped_packages:_excluded_test_setup INFO: Analyzed target //tests/untyped_packages:_excluded_test_setup (1 packages loaded, 2 targets configured, 1 aspect application). [... 3 lines ...] INFO: Build completed successfully, 5 total actions [... 1 lines ...] # exit=0 and with the `import()` beside it the setup compile fails the way the test file's does (15-red-setup-compile-no-attr.log): $ bazelisk --output_base=/var/tmp/fix8-P-ob build --noshow_progress //tests/untyped_packages:_excluded_test_setup INFO: Analyzed target //tests/untyped_packages:_excluded_test_setup (0 packages loaded, 0 targets configured). ERROR: /home/mikn/devel/lovable/rt-fix-p/tests/untyped_packages/BUILD.bazel:270:8: TsgoDeclare //tests/untyped_packages:_excluded_test_setup failed: (Exit 1): tsc failed: error executing TsgoDeclare command (from ts_compile rule target //tests/untyped_packages:_excluded_test_setup) external/+ts+tsgo_linux_amd64/lib/tsc --project bazel-out/k8-fastbuild/bin/tests/untyped_packages/_excluded_test_setup.tsconfig.json [... 2 lines ...] tests/untyped_packages/excluded_setup.ts(4,15): error TS2769: No overload matches this call. The last overload gave the following error. Argument of type 'HTMLDivElement' is not assignable to parameter of type 'string | ReadableStream<any> | Response'. [... 3 lines ...] ERROR: Build did NOT complete successfully [... 1 lines ...] # exit=1 Green with the change on tree d3805a8 (16-green-package.log): $ bazelisk --output_base=/var/tmp/fix8-P-ob test --noshow_progress --nocache_test_results --local_test_jobs=2 --test_summary=short //tests/untyped_packages/... //tools/changelog:changelog_test INFO: Analyzed 36 targets (2 packages loaded, 85 targets configured, 187 aspect applications). INFO: Found 14 targets and 22 test targets... [... 10 lines ...] //tests/untyped_packages:excluded_test PASSED in 0.6s [... 3 lines ...] //tests/untyped_packages:paths_keep_untyped_test PASSED in 0.0s [... 1 lines ...] //tests/untyped_packages:paths_omit_untyped_test PASSED in 0.0s [... 1 lines ...] //tests/untyped_packages:setup_paths_omit_untyped_test PASSED in 0.0s //tests/untyped_packages:test_paths_keep_untyped_test PASSED in 0.0s //tests/untyped_packages:test_paths_omit_untyped_test PASSED in 0.0s [... 2 lines ...] //tests/untyped_packages:untyped_packages_test PASSED in 0.0s //tools/changelog:changelog_test PASSED in 0.0s [... 1 lines ...] Executed 22 out of 22 tests: 22 tests pass. [... 2 lines ...] # exit=0 The same targets pass on this commit's tree, 97962f0, on a fresh output base: 22 of 22 (rt-evidence/logs/fix9r/P/01-green-package.log). docs/rules/ts-test.md gains the attribute row and a section beside the other forwarded compile attributes, and its node:test paragraph and the `runner` docstring entry name the attribute in their lists of the compile attributes that carry over; changelog.d/ts-test-untyped-packages.md records the addition, and tools/ci/check_test_sources.sh lists leaks.test.ts as manual-only.
This was referenced Sep 6, 2026
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.
ts_compile'suntyped_packageskeeps a global-script package out of one target's type program, and the leak it exists for arrives through a dep's npm closure: the tsconfig writer (ts/private/ts_compile.bzl, the deps loop overdep_npm_closure_sets) offers every package in every first-party dep's transitive npm closure apathskey, so a.d.tsimport of a package only a devDependency declared resolves in the sandbox where pnpm's layout leaves it unresolved, and the global script it names enters the program during import walking, ahead of thetypesentries, and its declarations win first-declaration over theirs. Ats_test's program is written from the samedepsby the same writer, and the macro (ts/private/ts_test.bzl) had nountyped_packagesparameter and forwarded none, so a test with the leak had no way to say it. On the trial monorepo that is//workers/proxy-worker2/test:_test_test_compile: 598error TSlines, of which 32 no run of the program's own tsconfig produces (logs/inv7/proxy-worker2-test/compare.log: 562 common, 38 Bazel-only, 6 of them message-text variants of common lines; the 32 are 14Property 'LOADER'/'DWL_KV'does not exist on type 'Env', 16does not exist on type 'Exports', 1Property 'getSetCookie' does not exist on type 'Headers'and the 1TS7006it causes), every one of them@cloudflare/workers-types@4.20260420.1reached through//workers/dwl-usage-consumer/srcand imported by@flarelabs-net/workers-observability-utils@0.3.4/dist/tail.d.ts(summary.log,bazel-tsconfig-execroot-explainfiles.log), declaringHeaderswithoutgetSetCookieandCloudflare.Envwithout the bindings the wrangler-generatedworker-configuration.d.tsintypesdeclares. The lane's probe cuts the generatedpathsmap down: with only the two@cloudflare/workers-typeskeys the four probe files keep 5Env, 12Exportsand 1getSetCookiediagnostics, and with those two keys removed they keep 0, 0 and 0 (probe/paths-bisect.log,only-wtagainstminus-wt); withpathsemptied the whole program keeps none of the 32 (probe/bisect.log). The trial already names the package on//workers/proxy-worker2/src/shared(untyped_packages = ["@cloudflare/workers-types"], itsBUILD.bazelline 13), ats_compile; the test beside it could not.ts_testtakesuntyped_packages = Noneand forwards it to everyts_compileit generates: the one oversrcs, and the ones_compile_setup_sourcesmakes over the TypeScript entries ofsetup_filesandglobal_setup, which compile with the samedepsand so with the same closure. The rule's two refusals apply unchanged on each of them: an entry naming no package in the compile's closure (_untyped_names) and a package named in bothuntyped_packagesandcompilerOptions.types(_fail_on_untyped_conflict).Docs:
docs/rules/ts-test.mdgains the attribute row and a section, "Keeping a Package Out of the Program", after the one on atypesentry that names a declaration file, pointing atts-compile.md's section of the same name; thets_testdocstring gains theuntyped_packagesentry, and the two lists of compile attributes that carry over to node:test, therunnerdocstring entry's and the page's own paragraph under "The node:test Runner", name it.changelog.d/ts-test-untyped-packages.mdrecords the addition under Added.tools/ci/check_test_sources.shliststests/untyped_packages/leaks.test.tsinMANUAL_ONLYwith its reason.Fixture
//tests/untyped_packagesis the package that pins the attribute onts_compile, over@npm_workers//:wrangler, whose declaration entry imports@cloudflare/workers-types; it gains the same pair on the programts_testgenerates.leaks_test(manual) isleaks.tsas a vitest file --Element.append(HTMLDivElement)and avoid import("wrangler")-- withdeps = ["@npm_workers//:vitest", "@npm_workers//:wrangler"];excluded_testis the same file withuntyped_packages = ["@cloudflare/workers-types"]and asetup_filesentry,excluded_setup.ts, carrying the same DOM call and the samevoid import("wrangler"), so the setup compile is an assertion too (below). Three analysis tests reuseuntyped_tests.bzl's existing rules on the generated compiles:test_paths_keep_untyped_testreads:_leaks_test_compile's tsconfig and asserts the@cloudflare/workers-typesand@cloudflare/workers-types/*keys are there;test_paths_omit_untyped_testreads:_excluded_test_compile's and asserts no key resolves the package whilewranglerstill does;setup_paths_omit_untyped_testasserts the same of:_excluded_test_setup. The twots_testtargets are named after their half rather than after the directory: Gazelle's<dir>_testname,untyped_packages_test, is the package's existingbuild_test, and a hand-writtents_testunder any other name keeps itssrcsclaimed (claimedSrcs,gazelle/generate.go), so the fixture adds nothing to Gazelle's diff (below).Red on 2bcc23f with
leaks_testand its analysis test in the tree and the.bzluntouched, tree 528c24f (logs/fix8r/P/01-red-leaks-test-compile.log): the ts_test's program leaks the way:leaksdoes.test_paths_keep_untyped_testpasses on that tree (02-base-keep-control.log,Executed 1 out of 1 test: 1 test passes.): the pair's control holds before the change. Withexcluded_testand its two analysis tests added, tree 324ba48 (03-red-package.log), the package does not load:The setup compile is an assertion only because
excluded_setup.tsloads wrangler's declaration entry too. Withuntyped_packagesstripped fromexcluded_test, a setup file carrying the DOM call alone compiles, since nothing in it reaches the global script (14-control-setup-compile-no-attr.log, tree 724a36f):and with the
import()beside it, still without the attribute,:_excluded_test_setupfails the way:_leaks_test_compiledoes (15-red-setup-compile-no-attr.log, tree f143e70):Green with the change, tree d3805a8, which is 939b90d's tree (
16-green-package.log):excluded_testcompiles and runs, and the three analysis tests read what they expect.On 939b90d with a clean tree,
bazelisk test //... --nocache_test_results --local_test_jobs=2on the same output base printsINFO: Found 520 targets and 374 test targets...andExecuted 374 out of 374 tests: 374 tests pass.in 223.4 s (24-suite.log); the four over the 370 at 2bcc23f (logs/restack-3/s06-skeptic-suite.log) areexcluded_testand the three analysis tests,leaks_testbeingmanual.bazelisk build --config=ci //... --output_groups=+_validationanalyzes 894 targets and completes (25-ci-build-validation.log, exit 0).bazel run //gazelle -- -mode=diffover tree d3805a8 prints 595 lines, byte-identical to the run on 2bcc23f's tree (17-modediff.stdoutagainstlogs/restack-3/s08-skeptic-modediff.stdout,cmpexit 0 in17-modediff.log;-mode=diffexits 1 on both, having diffs to print), and 0 of those lines mentionuntyped_packages.On 97962f0, this PR's tree,
buildifier --mode=check -r .exits 0 (logs/fix9r/P/02-lint-buildifier.log),gofmt -l .prints nothing (03-lint-gofmt.log),go vetover CI's package list exits 0 (04-lint-govet.log), anduvx --from mkdocs-material mkdocs build --strictbuilds the site (05-lint-mkdocs.log, exit 0; the section's one cross-reference ists-compile.md#keeping-a-package-out-of-the-program, the anchor of that page's existing heading).tools/ci/check_test_sources.shreports134 test source files, all claimed by a test target (6 manual-only, allowlisted)(06-check_test_sources.log):excluded.test.tsis claimed by a target that runs, andleaks.test.tsis the sixthMANUAL_ONLYentry.tools/ci/check_integration_shards.shreports15 tests in //tests/integration over 2 legs, each on exactly one(npm 5, core 10;07-check_integration_shards.log).3aa600b, this PR's head, is eb947cd with its commit message rewritten and its tree unchanged:
git diff --stat eb947cd 3aa600bprints nothing, and both trees are 97962f0 (logs/fix9r/P/08-amend.log). eb947cd is 939b90d rebased onto 888a7e3 with the patch unchanged:git range-diffprints!on thedocs/rules/ts-test.mdhunk header alone, whose context line #157 rewrote, and on no content line (logs/fix8r/restack/07-rebase-P.log). The targets the green block names pass on 97962f0 on a fresh output base:INFO: Found 14 targets and 22 test targets...,Executed 22 out of 22 tests: 22 tests pass.(logs/fix9r/P/01-green-package.log). The full suite at d3c9520, the stack's tip after the restack onto 888a7e3, is in #160's body (logs/fix8r/restack/12-suite.log, 380 of 380).On the trial
Nothing in lov-paraglide is edited by this PR. With the override at 3aa600b,
//workers/proxy-worker2/test/BUILD.bazel'stest_testcan carryuntyped_packages = ["@cloudflare/workers-types"], the line//workers/proxy-worker2/src/sharedalready carries; the package is in the test's closure through//workers/proxy-worker2/src/sharedand the othersrcdeps, so_untyped_namesaccepts it. The expectation is that//workers/proxy-worker2/test:_test_test_compiledrops the 32 Bazel-only diagnostics (compare.log) -- thepathskey is what the attribute removes, andprobe/bisect.logmeasured the program at 0 of them withpathsemptied -- and keeps the 566 the program's own tsconfig reproduces (own-tsconfig-generated-types.log: 568, of which 2 sit intest/setup.tson the sibling:testts_compile), which are the monorepo's test code, grouped ingroups.log. Whether the count moves, and by how much, is the trial's to measure.Base branch:
mikn/ts-test-runtime-npm-closure(00aa5cf); head 1edaa40, the polished commit 3aa600b rebased onto it with its patch unchanged (logs/fix9r/restack/restack-mechanical.log).