You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
perf: finish Group A — wire async URL resolution into watch + packages, then drop sync helpers
Follow-up to #3103 covering the two remaining PRs of the async-git stack. Both are smaller than the load-bearing #3109, but they're necessary for correctness (PR-3) and cleanup (PR-4).
This is a self-contained pickup brief — someone (me or another contributor) should be able to execute both PRs from this issue alone.
After #3109 lands, Application.convert() correctly awaits the new resolveDeferredSourceUrls() step. But two execution modes bypass Application.convert() entirely and call this.converter.convert() directly:
Watch mode — Application.convertAndWatch calls this.converter.convert() inside runSuccess (around line 659 of src/lib/application.ts).
Packages mode — Application._convertPackages is async but invokes per-package conversion through this.convert() for each package. Each package's convert() runs resolveDeferredSourceUrls() correctly, but the post-merge step does not.
Without PR-3, these modes ship reflections with source.url === undefined.
After PR-3, PR-4 is safe — every in-tree caller is on the async surface, and the sync git() helper, gitIsInstalled(), GitRepository.tryCreateRepository, RepositoryManager.getRepository, and RepositoryManager.getRepositoryFolder can be removed.
PR-3 — wire watch + packages modes into the new async step
Spec
Files to modify:src/lib/application.ts only.
Application.convertAndWatch (around line 503-687): in the runSuccess inner function (defined around line 626), find the spot where const project = this.converter.convert(entryPoints); is called (around line 659). Add an await this.converter.resolveDeferredSourceUrls() between that line and the existing void success(project).then(...) block.
The existing structure looks roughly like (line numbers approximate — verify against current state of master + #3109):
(Or restructure with an explicit async IIFE — whichever reads cleaner to the implementer. The semantic requirement: resolveDeferredSourceUrls() must complete before success(project) runs, since the success callback typically calls validate() and generateOutputs(), both of which read source.url.)
Application._convertPackages (around line 797-913): this method calls await this.convert() for each package (line ~871), so per-package conversion already gets the new step. The hazard is the merged result at the end (around line 901-911): the result from this.deserializer.reviveProjects(...) and this.converter.addProjectDocuments(result) does NOT go through resolveDeferredSourceUrls. But — none of the per-package merged reflections need fresh URL resolution because each package's source.url was already populated in its own convert() call. Verify this by reading reviveProjects and confirming it preserves source.url from the input project JSON.
If reviveProjects does NOT preserve source.url (e.g., if it strips it), then _convertPackages needs to call resolveDeferredSourceUrls() after the merge too. Investigation step before code change.
Tests
The existing tests cover convert mode well (1051 passing on mocha.fast after #3109). Watch and packages modes have less test coverage. Add at minimum:
Smoke test for watch mode: start typedoc with --watch --options .config/typedoc.json, modify a source file, confirm rebuilt HTML has source-link URLs. This is a manual smoke unless typedoc has a watch-mode test harness already (search src/test/ for watch to find out).
Packages mode test: if src/test/packages.test.ts already exists (probably does — it's in the fast suite), confirm it still passes and that the packages it produces have source.url set on at least one reflection. If no such assertion exists, add one.
Commit + open
git add src/lib/application.ts
git commit -m "perf(converter): wire async URL resolution into watch and packages modes"
Base branch: master (after #3104 and #3109 have merged). Until then, base on perf_26-05-26_async-url-defer (the #3109 branch on StoneCypher/typedoc).
PR-4 — drop the sync git helpers
Spec
Files to modify:
src/lib/converter/utils/repository.ts
src/test/Repository.test.ts
src/lib/converter/plugins/SourcePlugin.ts (potentially — if any sync import lingers)
Pre-flight grep
Before deleting anything, confirm nothing in production code still calls the sync helpers:
All async siblings (gitAsync, gitIsInstalledAsync, tryCreateRepositoryAsync, getRepositoryAsync)
RepositoryManager's constructor and the assumedRepo field
In Repository.test.ts:
Delete tests covering the sync surface (RepositoryManager - git enabled's sync test cases, tryCreateRepository direct tests, etc.). Keep the async versions and guessSourceUrlTemplate tests.
After deletion, run the full mocha fast suite. Expected: ~same passing count as after #3109 (the test count drops by however many sync-only tests were removed).
git add src/lib/converter/utils/repository.ts src/test/Repository.test.ts
git commit -m "refactor(converter): drop sync git helpers superseded by async API"
Base on PR-3's branch (or master, if PR-3 has merged by then).
Environmental notes (for whoever picks this up)
These are quirks the earlier PRs in the stack ran into. Worth knowing upfront.
NODE_OPTIONS="--use-system-ca" must prefix any corepack pnpm / npm / network-fetching Node command on Windows machines with custom corporate root CAs. Without it: UNABLE_TO_VERIFY_LEAF_SIGNATURE.
pnpm install --ignore-scripts is the only way to install in a hurry (the @typestrong/fs-fixture-builder git-hosted devDep has a prepare script that calls yarn install, which fails if yarn isn't on PATH). With --ignore-scripts, fs-fixture-builder's declarations are missing, so any test file that imports it will fail to compile via tsc but tsx (mocha's loader) tolerates it at import time as long as the test file in question doesn't import it.
Build fs-fixture-builder by hand when you need to run the full mocha suite. Recipe: write node_modules/.pnpm/@typestrong+fs-fixture-buil_*/node_modules/@typestrong/fs-fixture-builder/src/tsconfig-inline.json with {"compilerOptions": {"lib": ["es2023"], "module": "node16", "moduleResolution": "node16", "target": "es2022", "strict": true, "esModuleInterop": true, "skipLibCheck": true, "rootDir": ".", "outDir": "../dist", "types": ["node"], "newLine": "lf"}} then run tsc against it from that package directory.
dprint has a similar "postinstall skipped" issue. Recovery: node <worktree>/node_modules/dprint/install.cjs downloads the binary. After that, node <worktree>/node_modules/dprint/bin.cjs --config <worktree>/dprint.json check <files> works.
tsconfig.measure.json (or tsconfig.bundle.json) — a one-off tsconfig that extends ./tsconfig.json and excludes src/test is the cleanest way to get a dist/ build when fs-fixture-builder isn't built. Used by PR-2 and PR-8.
bin/typedoc is CJS (per bin/package.json"type": "commonjs" override) even though the top-level package is ESM. Use require() / __dirname, not import / import.meta.url.
Verification before push
Both PRs must clear:
ESLint --max-warnings 0 scoped to changed files (CI will run unscoped; pre-existing fs-fixture-builder typing complaints in tests are unrelated noise from your hand-built version)
dprint check on changed files
mcp__ide__getDiagnostics clean on changed files (TypeScript-only — won't catch @typescript-eslint/restrict-plus-operands-style strict-lint issues, so don't rely on this alone)
perf: finish Group A — wire async URL resolution into watch + packages, then drop sync helpers
Follow-up to #3103 covering the two remaining PRs of the async-git stack. Both are smaller than the load-bearing #3109, but they're necessary for correctness (PR-3) and cleanup (PR-4).
This is a self-contained pickup brief — someone (me or another contributor) should be able to execute both PRs from this issue alone.
Status of the stack as of this writing
Group B PRs (#3105, #3106, #3107, #3108) are independent of this work.
Why these two remain
After #3109 lands,
Application.convert()correctly awaits the newresolveDeferredSourceUrls()step. But two execution modes bypassApplication.convert()entirely and callthis.converter.convert()directly:Application.convertAndWatchcallsthis.converter.convert()insiderunSuccess(around line 659 ofsrc/lib/application.ts).Application._convertPackagesis async but invokes per-package conversion throughthis.convert()for each package. Each package'sconvert()runsresolveDeferredSourceUrls()correctly, but the post-merge step does not.Without PR-3, these modes ship reflections with
source.url === undefined.After PR-3, PR-4 is safe — every in-tree caller is on the async surface, and the sync
git()helper,gitIsInstalled(),GitRepository.tryCreateRepository,RepositoryManager.getRepository, andRepositoryManager.getRepositoryFoldercan be removed.PR-3 — wire watch + packages modes into the new async step
Spec
Files to modify:
src/lib/application.tsonly.Application.convertAndWatch(around line 503-687): in therunSuccessinner function (defined around line 626), find the spot whereconst project = this.converter.convert(entryPoints);is called (around line 659). Add anawait this.converter.resolveDeferredSourceUrls()between that line and the existingvoid success(project).then(...)block.The existing structure looks roughly like (line numbers approximate — verify against current state of master + #3109):
Replace with:
(Or restructure with an explicit
asyncIIFE — whichever reads cleaner to the implementer. The semantic requirement:resolveDeferredSourceUrls()must complete beforesuccess(project)runs, since the success callback typically callsvalidate()andgenerateOutputs(), both of which readsource.url.)Application._convertPackages(around line 797-913): this method callsawait this.convert()for each package (line ~871), so per-package conversion already gets the new step. The hazard is the merged result at the end (around line 901-911): theresultfromthis.deserializer.reviveProjects(...)andthis.converter.addProjectDocuments(result)does NOT go throughresolveDeferredSourceUrls. But — none of the per-package merged reflections need fresh URL resolution because each package'ssource.urlwas already populated in its ownconvert()call. Verify this by readingreviveProjectsand confirming it preservessource.urlfrom the input project JSON.If
reviveProjectsdoes NOT preservesource.url(e.g., if it strips it), then_convertPackagesneeds to callresolveDeferredSourceUrls()after the merge too. Investigation step before code change.Tests
The existing tests cover convert mode well (1051 passing on
mocha.fastafter #3109). Watch and packages modes have less test coverage. Add at minimum:Smoke test for watch mode: start typedoc with
--watch --options .config/typedoc.json, modify a source file, confirm rebuilt HTML has source-link URLs. This is a manual smoke unless typedoc has a watch-mode test harness already (searchsrc/test/forwatchto find out).Packages mode test: if
src/test/packages.test.tsalready exists (probably does — it's in the fast suite), confirm it still passes and that the packages it produces havesource.urlset on at least one reflection. If no such assertion exists, add one.Commit + open
Base branch: master (after #3104 and #3109 have merged). Until then, base on
perf_26-05-26_async-url-defer(the #3109 branch onStoneCypher/typedoc).PR-4 — drop the sync git helpers
Spec
Files to modify:
src/lib/converter/utils/repository.tssrc/test/Repository.test.tssrc/lib/converter/plugins/SourcePlugin.ts(potentially — if any sync import lingers)Pre-flight grep
Before deleting anything, confirm nothing in production code still calls the sync helpers:
grep -rn "tryCreateRepository\b\|gitIsInstalled\b\|RepositoryManager\b" src/lib/Every match must be one of:
repository.ts)tryCreateRepositoryAsync,gitIsInstalledAsync— these have the suffix, so\bboundary excludes them)RepositoryManageritself (the class name still stays — only the sync methods on it go)If anything else matches, that caller needs to migrate first.
What to delete
In
repository.ts:git(...args)helper (currently around lines 9-15)gitIsInstalled()function (currently around lines 17-21)GitRepository.tryCreateRepositorystatic method (the sync one)RepositoryManager.getRepositorymethodRepositoryManager.getRepositoryFolderprivate methodWhat stays:
AssumedRepository.getURLandGitRepository.getURL(no git invocation)GitRepository's constructor (already data-only after perf(converter): add async git infrastructure #3104)guessSourceUrlTemplate(pure function)gitAsync,gitIsInstalledAsync,tryCreateRepositoryAsync,getRepositoryAsync)RepositoryManager's constructor and theassumedRepofieldIn
Repository.test.ts:RepositoryManager - git enabled's sync test cases,tryCreateRepositorydirect tests, etc.). Keep the async versions andguessSourceUrlTemplatetests.In
SourcePlugin.ts:gitIsInstalledlingers, remove it. (After perf(converter): resolve source URLs in parallel after conversion ends #3109 it should already be gone; double-check.)Tests
After deletion, run the full mocha fast suite. Expected: ~same passing count as after #3109 (the test count drops by however many sync-only tests were removed).
Also re-run lint:
And dprint:
Commit + open
Base on PR-3's branch (or master, if PR-3 has merged by then).
Environmental notes (for whoever picks this up)
These are quirks the earlier PRs in the stack ran into. Worth knowing upfront.
NODE_OPTIONS="--use-system-ca"must prefix anycorepack pnpm/npm/ network-fetching Node command on Windows machines with custom corporate root CAs. Without it:UNABLE_TO_VERIFY_LEAF_SIGNATURE.pnpm install --ignore-scriptsis the only way to install in a hurry (the@typestrong/fs-fixture-buildergit-hosted devDep has apreparescript that callsyarn install, which fails if yarn isn't on PATH). With--ignore-scripts,fs-fixture-builder's declarations are missing, so any test file that imports it will fail to compile via tsc but tsx (mocha's loader) tolerates it at import time as long as the test file in question doesn't import it.node_modules/.pnpm/@typestrong+fs-fixture-buil_*/node_modules/@typestrong/fs-fixture-builder/src/tsconfig-inline.jsonwith{"compilerOptions": {"lib": ["es2023"], "module": "node16", "moduleResolution": "node16", "target": "es2022", "strict": true, "esModuleInterop": true, "skipLibCheck": true, "rootDir": ".", "outDir": "../dist", "types": ["node"], "newLine": "lf"}}then run tsc against it from that package directory.node <worktree>/node_modules/dprint/install.cjsdownloads the binary. After that,node <worktree>/node_modules/dprint/bin.cjs --config <worktree>/dprint.json check <files>works.tsconfig.measure.json(ortsconfig.bundle.json) — a one-off tsconfig that extends./tsconfig.jsonand excludessrc/testis the cleanest way to get adist/build whenfs-fixture-builderisn't built. Used by PR-2 and PR-8.bin/typedocis CJS (perbin/package.json"type": "commonjs"override) even though the top-level package is ESM. Userequire()/__dirname, notimport/import.meta.url.Verification before push
Both PRs must clear:
--max-warnings 0scoped to changed files (CI will run unscoped; pre-existing fs-fixture-builder typing complaints in tests are unrelated noise from your hand-built version)mcp__ide__getDiagnosticsclean on changed files (TypeScript-only — won't catch@typescript-eslint/restrict-plus-operands-style strict-lint issues, so don't rely on this alone)Tracking
Blocks #3103.
Once PR-3 and PR-4 are open, link them in the parent issue's PR stack table.