Skip to content

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

Draft
cryptodev-2s wants to merge 10 commits into
mainfrom
refactor/platform-api-docs-ts-morph-globbing
Draft

refactor(platform-api-docs): use ts-morph globbing for file discovery#9990
cryptodev-2s wants to merge 10 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

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
* @param root - Resolved directory the positive pattern is rooted at.
* @returns The exclusion patterns.
*/
function tsSourceExclusions(root: string): string[] {

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.

Since this is a function, should its name start with a verb?

Suggested change
function tsSourceExclusions(root: string): string[] {
function buildTsSourceExclusions(root: string): string[] {

* @param root - Resolved `node_modules/@metamask` directory.
* @returns The exclusion patterns.
*/
function declarationFileExclusions(root: string): string[] {

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.

Suggested change
function declarationFileExclusions(root: string): string[] {
function buildDeclarationFileExclusions(root: string): string[] {

Base automatically changed from feat/platform-api-docs-root-messenger-strategy to main August 27, 2026 21:02
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.

2 participants