Skip to content

refactor(platform-api-docs): use ts-morph globbing for file discovery - #9990

Merged
cryptodev-2s merged 22 commits into
mainfrom
refactor/platform-api-docs-ts-morph-globbing
Sep 2, 2026
Merged

cryptodev-2s merged 22 commits into
mainfrom
refactor/platform-api-docs-ts-morph-globbing

Conversation

@cryptodev-2s

@cryptodev-2s cryptodev-2s commented Aug 27, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #9913, targets its branch.

Replaces hand-rolled file discovery with Project#addSourceFilesAtPaths, which takes glob patterns directly. Drops discovery.ts, the per-file add/extract loops, listTargetSubdirectories, and the glob dependency. Also merges the two Project factories (they differed by one field) and removes the redundant getSourceFile() guard, closing #9913 (comment).

Net 254 lines deleted. Generation over this monorepo: 114s to 8.6s.

Output is byte identical. diff -r on full docs trees generated before and after, for core, mobile and extension: all empty, sidebars included.

Two traps, if you touch these patterns

Both produced wrong output while the whole suite stayed green.

  • Glob roots need realpath. The matcher does not follow a symlinked ancestor. Under /var/folders/... the declaration glob returned 0 where /private/var/... returned 1980, so any project under /tmp or /var would have emitted empty docs.
  • Negations must be anchored. !**/*.test.ts resolves against the process cwd, not against the pattern it accompanies. Since yarn workspace sets cwd to the package dir, every exclusion silently stopped matching and test fixtures leaked in (117 namespaces instead of 112).

Related: declaration-file exclusions are deliberately narrower than the source ones. A blanket dist/** matches the segment those files live under and drops all 1980.

Behaviour change

A broken symlink used to emit a per-file parse warning. The matcher now resolves it away before ts-morph sees it, so it is skipped silently; valid files are still documented. One test rewritten to match.

Follow-up

Second step makes each strategy self contained, which is what #9913 (comment) asked for. Worth knowing before that review: extraction.ts is a scan-only front end welded onto a shared core, so the split has to cut through it too, not just generate.ts.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

Low Risk
Internal tooling refactor with byte-identical output claimed; main risk is glob/exclusion edge cases on unusual filesystem layouts, now covered by new tests.

Overview
Speeds up doc generation by replacing per-file glob discovery and addSourceFileAtPath loops with a single shared ts-morph Project that bulk-loads matches via addSourceFilesAtPaths. Removes discovery.ts, the glob dependency, and merges the two project factories into shared createProject() in ts-project.ts.

The scan strategy now builds resolved, forward-slashed glob roots (toGlobPath + realpath) and content-root–anchored exclusion negations so test/dist skips work when cwd ≠ project root and monorepo packages named test/dist are not excluded wholesale. Added source files are sorted by path for stable dedup/source links; extraction uses a realpath’d project root for relative source paths.

Behavior: unreadable paths (e.g. broken symlinks) are skipped without per-file parse warnings; valid files still document. Tests cover the packages/test exclusion edge case and the symlink case.

Reviewed by Cursor Bugbot for commit 55c4a26. Bugbot is set up for automated code reviews on this repo. Configure here.

cryptodev-2s and others added 8 commits August 20, 2026 14:41
## Explanation

<!--
Thanks for your contribution! Take a moment to answer these questions so
that reviewers have the information they need to properly understand
your changes:

* What is the current state of things and why does it need to change?
* What is the solution your changes offer and how does it work?
* Are there any changes whose purpose might not obvious to those
unfamiliar with the domain?
* If your primary goal was to update one package but you found you had
to update another one along the way, why did you do so?
* If you had to upgrade a dependency, why did you do so?
-->

- Refactor and clean up `cli.ts`
- Update `cli.ts` to extract `parseArguments`, further clean `main`
- Revise `root-messenger-discovery.ts` (update terminology, JSDoc)
- Simplify `{Scan,RootMessenger}StrategyOptions`, enforce that
`generate` takes strategy

## References

<!--
Are there any issues that this pull request is tied to?
Are there other links that reviewers should consult to understand these
changes better?
Are there client or consumer pull requests to adopt any breaking
changes?

For example:

* Fixes #12345
* Related to #67890
-->

## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md)
- [ ] I've introduced [breaking
changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md)
in this PR and have prepared draft pull requests for clients and
consumer packages to resolve them

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Programmatic callers of `generate` must pass `strategy` (breaking for
library use); root-messenger discovery is heavily renamed but CLI-facing
behavior should stay the same.
> 
> **Overview**
> Refactors **platform-api-docs** around clearer typing and naming for
the scan vs **root-messenger** discovery paths, without changing CLI
flags.
> 
> **CLI (`cli.ts`)** pulls argument parsing into **`parseArguments`**,
models options as a discriminated union (`ParsedArguments`), and has
**`checkStrategyArgs`** return normalized scan or root-messenger args
instead of only validating. **`--root-actions`** / **`--root-events`**
are coerced via **`parseRootCapabilitiesTypeReference`** at parse time;
**`main`** calls **`generate`** in separate branches with typed options.
> 
> **`generate`** now **requires** an explicit **`strategy: 'scan' |
'root-messenger'`** on **`GenerateOptions`** (scan and root-messenger
option shapes are a strict union). Scan tests pass **`strategy:
'scan'`** accordingly.
> 
> **Root-messenger discovery** renames and restructures APIs:
**`RootTypeReference`** → **`RootCapabilitiesTypeReference`**,
**`parseRootTypeReference`** → **`parseRootCapabilitiesTypeReference`**,
**`discoverFromRootMessenger`** →
**`discoverFromRootMessengerCapabilitiesTypes`**, with result fields
**`capabilityPackets`** / **`skippedCapabilities`**
(**`unnamedCapabilities`**, **`unextractableCapabilities`**). Internal
helpers use object parameters and expanded JSDoc; extraction behavior is
unchanged.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
4f7745c. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@cryptodev-2s
cryptodev-2s requested a review from a team as a code owner August 27, 2026 15:10
@cryptodev-2s
cryptodev-2s marked this pull request as draft August 27, 2026 15:12
@cryptodev-2s cryptodev-2s self-assigned this Aug 27, 2026
@cursor
cursor Bot requested review from Mrtenz and mcmire August 27, 2026 15:20
@cryptodev-2s
cryptodev-2s force-pushed the refactor/platform-api-docs-ts-morph-globbing branch from 0e0845d to dfd99a2 Compare August 27, 2026 15:48
Replaces the hand rolled file walking with addSourceFilesAtPaths, which
takes glob patterns directly. Deletes discovery.ts, the per file add and
extract loops, and the glob dependency. Collapses the two near identical
Project factories into one createProject(), and drops the redundant
getSourceFile() guard now that addSourceFileAtPath is known to be
idempotent.

Generated output is byte identical, verified by diffing full docs trees
built before and after for this monorepo, metamask-mobile and
metamask-extension. Generation over this monorepo drops from 114s to
8.6s, since files are added in bulk rather than one at a time.

Two things worth knowing for anyone touching these patterns:

Glob roots are resolved with realpath first. The matcher does not follow
a symlinked ancestor of a pattern, so on macOS a project under /tmp or
/var would otherwise match nothing at all.

Exclusions are anchored to their pattern's root. An unanchored
!**/*.test.ts resolves against the working directory, so it silently
stops excluding anything when the scanned path is elsewhere, which is
the normal case when invoked via yarn workspace.
@cryptodev-2s
cryptodev-2s force-pushed the refactor/platform-api-docs-ts-morph-globbing branch from dfd99a2 to 4734579 Compare August 27, 2026 15:51
Comment thread packages/platform-api-docs/src/generate.ts Outdated
Comment thread packages/platform-api-docs/src/generate.ts Outdated
Base automatically changed from feat/platform-api-docs-root-messenger-strategy to main August 27, 2026 21:02
@cryptodev-2s
cryptodev-2s marked this pull request as ready for review August 28, 2026 08:08
…-docs-ts-morph-globbing

# Conflicts:
#	packages/platform-api-docs/CHANGELOG.md
#	packages/platform-api-docs/src/generate.test.ts
#	packages/platform-api-docs/src/generate.ts
#	packages/platform-api-docs/src/root-messenger-discovery.ts

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 4734579. Configure here.

Comment thread packages/platform-api-docs/src/generate.ts
@cursor
cursor Bot requested a review from mcmire August 28, 2026 08:15
Renames tsSourceExclusions and declarationFileExclusions to
buildTsSourceExclusions and buildDeclarationFileExclusions, so the names
read as the functions they are. Review feedback.
The exclusions were anchored at `packages` rather than at
`packages/*/src`, which made the first path segment a package name. A
workspace package called `test` or `dist` therefore matched `test/**`
and was dropped whole, where the previous implementation rooted its
ignores at each package's `src` and only skipped nested directories.

No effect on this repo, which has no package by those names, so add a
regression test that fails without the fix.
Sorting with localeCompare made the traversal order depend on the
locale the process runs under, and deduplication keeps the first of two
equally scored items, so the source link a capability gets could vary
by machine. Compares code units instead.

Note the order still differs from the previous implementation, which
enumerated package directories via readdir and so depended on the
filesystem. A global sort is deterministic where that was not. Docs
output over this repo is unchanged either way.
Comment thread packages/platform-api-docs/src/generate.ts Outdated
Comment thread packages/platform-api-docs/src/generate.ts
Comment thread packages/platform-api-docs/src/generate.ts Outdated
@cursor
cursor Bot requested a review from Mrtenz September 1, 2026 12:53

@mcmire mcmire left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried this out and it definitely speeds things up. Nice work! Just had a few comments below.

Comment thread packages/platform-api-docs/src/generate.ts Outdated
Comment thread packages/platform-api-docs/src/generate.ts Outdated
Comment thread packages/platform-api-docs/src/root-messenger-discovery.ts Outdated
Comment thread packages/platform-api-docs/src/generate.ts
@cryptodev-2s
cryptodev-2s requested a review from mcmire September 2, 2026 16:21

@mcmire mcmire left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@cryptodev-2s
cryptodev-2s requested review from Mrtenz and removed request for Mrtenz September 2, 2026 16:37
@cryptodev-2s
cryptodev-2s added this pull request to the merge queue Sep 2, 2026
Merged via the queue into main with commit 458aa0a Sep 2, 2026
436 checks passed
@cryptodev-2s
cryptodev-2s deleted the refactor/platform-api-docs-ts-morph-globbing branch September 2, 2026 17:55
pull Bot pushed a commit to Reality2byte/core that referenced this pull request Sep 23, 2026
…ating (MetaMask#10085)

## Explanation

A capability declared in a package's source is also visible in the
`dist` built from it, and a cross-package import resolves to that `dist`
rather than to the sibling's source. Whichever was reached first won, so
83 of 1164 source links pointed at `.d.cts` build output instead of code
you can read and edit.

Deduplication now scores source above build output, so the outcome no
longer depends on traversal order. All 1164 links point at source, and
namespace, action and event counts are unchanged.

Both clients generate byte-identical docs, since they only ever see
published packages and score every candidate the same way.

## References

* Follow up to MetaMask#9990, as promised in
MetaMask#9990 (comment)

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md)
- [ ] I've introduced [breaking
changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md)
in this PR and have prepared draft pull requests for clients and
consumer packages to resolve them

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Documentation generation and link selection only; no runtime API or
security behavior changes.
> 
> **Overview**
> Generated platform API docs were linking many capabilities to
**`dist`** declaration files (`.d.cts`) instead of editable **`.ts`
source**, because monorepo scans see the same action/event twice and
cross-package imports often resolve through **`node_modules`** to build
output first—whichever duplicate was processed first won.
> 
> **Deduplication scoring** now adds a **`sourceScore`** that favors
paths outside **`/dist/`**, so when both source and build declarations
exist, docs keep the source file for links and metadata.
Published-package-only consumers are unchanged (every candidate still
looks like **`dist`**).
> 
> Tests cover scanning with **`node_modules`** under a broad scan dir
and a monorepo-style **`a-controller` / `b-controller`** layout
asserting **`actions.md`** points at **`packages/b-controller/src/...`**
and not **`/dist/`**. The changelog records the fix under **Fixed**.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
38afbc9. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
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.

3 participants