feat: add @metamask/platform-api-docs package#8012
Conversation
cryptodev-2s
force-pushed
the
feat/messenger-docs-site
branch
4 times, most recently
from
63f3188 to
7c63a0d
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
|
Caution MetaMask internal reviewing guidelines:
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
cryptodev-2s
force-pushed
the
feat/messenger-docs-site
branch
3 times, most recently
from
b55b682 to
980f677
Compare
|
@metamaskbot publish-preview |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
cryptodev-2s
force-pushed
the
feat/messenger-docs-site
branch
from
980f677 to
f49a7dd
Compare
|
@metamaskbot publish-preview |
1 similar comment
|
@metamaskbot publish-preview |
cryptodev-2s
force-pushed
the
feat/messenger-docs-site
branch
from
f49a7dd to
44fb063
Compare
|
@metamaskbot publish-preview |
cryptodev-2s
changed the title
cryptodev-2s
changed the title
@metamask/messenger-docs
cryptodev-2s
changed the title
@metamask/messenger-docs@metamask/messenger-docs package
Docusaurus site for browsing controller messenger actions/events, with offline search powered by docusaurus-search-local.
## Explanation The messenger docs generation currently lives in `scripts/generate-messenger-docs/` and the Docusaurus site template in `docs-site/`. This makes it unusable by external clients (metamask-extension, metamask-mobile) without access to this monorepo. This PR extracts both into a new `@metamask/messenger-docs` package at `packages/messenger-docs/` with a CLI, so any project with `@metamask` controller dependencies can generate and serve messenger API docs. ### Usage ```bash # Default: scans cwd for node_modules/@MetaMask controller/service packages npx @metamask/messenger-docs # Scan a specific project npx @metamask/messenger-docs /path/to/project # Generate + build static site npx @metamask/messenger-docs --build # Generate + serve (build + http server) npx @metamask/messenger-docs --serve # Generate + dev server (hot reload) npx @metamask/messenger-docs --dev # Scan source .ts files instead of .d.cts (for monorepo development) npx @metamask/messenger-docs --source # Custom output directory (default: .messenger-docs) npx @metamask/messenger-docs --output ./my-docs ``` ## References - Builds on top of `feat/messenger-docs-site` ## 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
cryptodev-2s
force-pushed
the
feat/messenger-docs-site
branch
from
cb82c24 to
17adacd
Compare
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Make `site/tsconfig.json` extend the monorepo's shared `tsconfig.base.json`, keeping only the overrides Docusaurus actually needs (composite off, noEmit, JSX, ESNext modules, Bundler resolution). Drop the redundant `target`, `esModuleInterop`, and `strict` fields now that they come from base. Also stop excluding `packages/platform-api-docs/site/**` from ESLint so `site/docusaurus.config.ts` is type-checked as part of the normal lint flow rather than being an island only exercised by `yarn docs:platform-api:build`. typescript-eslint's `projectService: true` picks up the site's tsconfig automatically. Move `@docusaurus/types` from devDependencies to dependencies — `site/docusaurus.config.ts` imports `Config` from it, and `site/` is published as part of this package, so a consumer of the published CLI needs the types resolvable when Docusaurus loads the config at runtime.
Including `packages/platform-api-docs/site/**` in ESLint made typescript-eslint's `projectService` discover the site's tsconfig and try to type-check `docusaurus.config.ts`. Loading the full Docusaurus + React + webpack type graph blows past `lint:eslint`'s `--max-old-space-size=6144` cap and crashes CI with a heap-OOM. The `extends`-from-base wiring on `site/tsconfig.json` and the `@docusaurus/types` dependency move from eec9874 are kept — only the ESLint inclusion is reverted.
This reverts commit eec9874.
This reverts commit ed8d9d8.
The 6GB cap was set in #7906 (Feb 2026) when ESLint last started OOM-ing under typescript-eslint's typed rules. The monorepo has grown enough since that the cap is again insufficient — CI for this branch started OOM-ing after a routine merge with main brought in five more commits, despite no on-branch changes affecting lint. Bumping to 8GB.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| : 'Actions and events available for use in clients via the message bus', | ||
| url: process.env.DOCS_URL ?? 'https://metamask.github.io', | ||
| baseUrl: process.env.DOCS_BASE_URL ?? '/', | ||
| favicon: 'img/favicons/favicon-96x96.png', |
There was a problem hiding this comment.
Favicon references non-existent static file path
Low Severity
The Docusaurus config sets favicon to 'img/favicons/favicon-96x96.png', but the diff shows no favicons subdirectory or PNG file under site/static/img/ — only three SVG files exist there. Similarly, custom.css references a font at ../../static/fonts/MM-Sans/MM_Sans_Mono-Regular.woff2, but no fonts directory or WOFF2 file is included in the package. Both will produce broken references at build/runtime.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e583465. Configure here.
There was a problem hiding this comment.
Both files exist in the repo as tracked binary assets — they just don't surface in the PR diff:
$ git ls-files packages/platform-api-docs/site/static/
packages/platform-api-docs/site/static/fonts/MM-Sans/MM_Sans_Mono-Regular.woff2
packages/platform-api-docs/site/static/img/favicons/favicon-96x96.png
packages/platform-api-docs/site/static/img/metamask-fox.svg
packages/platform-api-docs/site/static/img/metamask-logo-dark.svg
packages/platform-api-docs/site/static/img/metamask-logo.svg
And yarn docs:platform-api:build copies them both into the static-site output (build/img/favicons/favicon-96x96.png, build/fonts/MM-Sans/MM_Sans_Mono-Regular.woff2) without any missing-asset warnings.
## 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? --> The goal of this commit is to make `extraction.ts` easier to follow. Since we are working with the TypeScript AST, we have to accept that the code will be somewhat dense, and we will need to ask maintainers to do some amount of studying to develop a mental model in their head. However, the more we can do to assist this process, the better. To this end, this commit: - Adds comments to illustrate steps in the parsing process and underlying nodes in the AST that are being visited - Renames some functions to more accurately describe their contents - Removes unnecessary `istanbul ignore` comments in favor of either adding tests that exercise the underlying logic or using assumptions (non-null assertions). - Rewrite some JSDoc so it's less technical and more natural sounding ## 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 - [x] 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** > Changes core AST extraction logic that drives published API docs; behavior should stay equivalent but edge-case handling shifted to type-checker resolution and explicit skips. > > **Overview** > Refactors **`platform-api-docs`** messenger extraction for readability and safer edge-case handling, without changing the public doc-generation pipeline’s overall shape. > > **`extraction.ts`** is reorganized into clearer phases: find `*Messenger` type aliases, recursively walk Actions/Events type parameters to collect capability declarations, then extract **`MessengerCapabilityPacket`** records (renamed from `ExtractedMessengerCapabilityType`) via inline object literals or `ControllerGetStateAction` / `ControllerStateChangeEvent` constructors. Discovery and extraction helpers are renamed and documented with AST-oriented comments; JSDoc handling is tightened (e.g. skip nameless `@param` tags, bare `@deprecated`). > > **Type resolution** for capability `type` strings and constructor namespace args now leans on the type checker (`getType().isStringLiteral()`) instead of dedicated template-literal / `typeof` helpers. Invalid shapes are skipped explicitly (missing `type`/`handler`/`payload`, numeric `type`, qualified `Ns.Type` references, non–string-literal constructor args, method signatures mixed into object types). > > **Tests** in `extraction.test.ts` add broad coverage for those skip/fallback paths; several `istanbul ignore` branches are removed in favor of tests or compile-time assumptions. > > **`generate.ts`**, **`markdown.ts`**, and **`types.ts`** only adopt the new type name. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 57ad0bd. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
|
@metamaskbot publish-preview |
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
The refactor in 0862ed5 renamed `ExtractedMessengerCapabilityType` to `MessengerCapabilityPacket` across the source files but missed the import + helper signatures in `markdown.test.ts`. The build didn't catch it (test files are excluded from `tsconfig.build.json`), Jest didn't catch it (Babel strips `import type` at compile time), and ESLint doesn't surface TS module-resolution errors — only `tsc --noEmit` against the dev tsconfig flagged the dangling reference.
The walker's bare-TypeReference branch — added to avoid double-documenting
plain re-exports like `type AllowedActions = ForeignAction` — was
swallowing the auto-generated `*MethodActions` alias whenever a
controller exposed exactly one bulk-registered method, e.g.
type DelegationControllerMethodActions =
DelegationControllerSignDelegationAction;
Since the body has no type arguments, the previous logic treated this as
a re-export and dropped the chain, so `signDelegation` never appeared in
the docs even though it was reachable from the messenger via
`DelegationControllerActions`.
Recurse into the referenced type instead. The shared `visitedTypeDeclarations`
set prevents the double-counting that the skip was originally guarding
against, so the same-file `SharedAction | AliasOfShared` case still
extracts exactly one capability. Cross-file re-exports continue to land
under their declaring file's namespace because
`extractFromMessengerCapabilityTypeDeclaration` uses the resolved
declaration's source file.
End-to-end extraction over the monorepo goes from 1062 items / 88
namespaces to 1259 items / 103 namespaces; DelegationController now
documents both `getState` and `signDelegation` as expected.
`tryToExtractFromCapabilityTypeConstructor` was re-checking that the declaration was a type alias and that its body was a `TypeReference` even though `recursivelyFindMessengerCapabilityTypeDeclarations` had already established both — the duplicate guards needed `// istanbul ignore next` annotations to keep coverage clean. The two TODOs left in 0862ed5 ("Capture the following information ahead of time so we don't need to check this again") flagged exactly this. Split `MessengerCapabilityTypeDeclaration` into a discriminated union of `bodyShape: 'constructor'` (type alias whose body is a `TypeReferenceNode` with type arguments) and `bodyShape: 'object'` (any other type alias or an interface). The walker classifies the body once when it captures the declaration; `extractFromMessengerCapabilityTypeDeclaration` dispatches on the tag instead of falling back through both extractors; each extractor consumes its specific variant and reads the pre-narrowed `body` directly. This drops both duplicate `isTypeReference` / `isTypeAliasDeclaration` guards in the constructor extractor (the istanbul-ignored ones); the remaining `!isIdentifier(typeName)` guard stays because `getTypeName()` returns `EntityName`, which a `TypeReferenceNode`'s type narrowing can't refine on its own. Added a regression test for the new walker path that handles exotic body shapes (intersections) — they now flow through `bodyShape: 'object'` and get rejected by the literal extractor's `isTypeLiteral` check. End-to-end extraction still produces 1259 items / 103 namespaces; coverage stays at 100/100/100/100.
| namespacePrefix.length > 0 && | ||
| item.sourceFile.toLowerCase().includes(namespacePrefix) | ||
| ? 1 | ||
| : 0; |
There was a problem hiding this comment.
Dedup home-package check uses overly broad substring match
Low Severity
The deduplicationScore function computes a "home" bonus by checking if item.sourceFile.toLowerCase().includes(namespacePrefix). For short namespace prefixes (e.g., namespace "App" → prefix "app", or "Log" → "log"), this matches unrelated file paths like packages/approval-controller/src/... or packages/dialog-utils/src/.... This can cause the wrong duplicate to win the tiebreaker, documenting a re-export instead of the home definition.
Reviewed by Cursor Bugbot for commit 09190bf. Configure here.
There was a problem hiding this comment.
This doesn't seem like a big deal to me. If there is a bug here we can fix it later.
The block at the call site for `resolveIndexedAccessMethod` is what makes the auto-generated `*-method-action-types.ts` flow produce rich Parameters tables: the aliases just reference `FooController['method']` without copying the JSDoc, so the action page would render an empty Parameters section without this inheritance. Replace the open TODO with a comment explaining the intent.
| @@ -0,0 +1,11 @@ | |||
| { | |||
| "extends": "../../../tsconfig.base.json", | |||
There was a problem hiding this comment.
Site tsconfig extends path breaks when copied to output
Low Severity
The "extends": "../../../tsconfig.base.json" path is correct from the source location at packages/platform-api-docs/site/, but setupSite copies this file via fs.cp into the output directory (e.g. <project>/.platform-api-docs/). From there, the three-level-up relative path resolves outside the project to a non-existent file. When used as a published npm package the path is even more wrong. Docusaurus likely still builds because jiti doesn't depend on this tsconfig, but the shipped file is broken.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1882f5c. Configure here.
There was a problem hiding this comment.
Right on both counts: Docusaurus uses `jiti` and never consults this tsconfig at runtime, but the copied/shipped file did have a broken `extends` path. Fixed in 7bb1849 by adding `tsconfig.json` to the `setupSite` skip set so we no longer copy it into the output directory (alongside the existing `node_modules` and `docs` skips). The source file keeps `extends: "../../../tsconfig.base.json"` so IDE / lint typecheck inheritance still works (per the earlier request at #discussion_r3277017046), without shipping a broken copy.
Action pages now reflect only the JSDoc on the type alias itself — description, `@param`, `@returns`, and `@deprecated` no longer fall through to the referenced class method when the alias has none. The handler signature substitution stays so `Class['method']` still renders as `(arg: T) => R` instead of the raw indexed-access syntax. In practice this is a no-op for the rendered docs: every controller that uses the `*-method-action-types.ts` bulk-registration pattern already carries the full JSDoc (description, `@param`, `@returns`) on the auto-generated type alias, and the hand-written `Class['method']` handlers in `PermissionController` likewise document each alias directly. NetworkController's `addNetwork` and DelegationController's `signDelegation` continue to render with populated Parameters tables. - Collapse `buildMethodInfo` into `buildMethodSignature` returning just the signature string; drop the `MethodInfo` type from `types.ts` and the import in `extraction.ts`. - Replace the test that locked in JSDoc inheritance with one that locks in its absence; delete the redundant alias-prefers-over-method test (alias-only is now the only path). Coverage stays at 100/100/100/100 and the end-to-end extraction still produces 1259 items / 103 namespaces.
`oxfmt` collapses the `import type { ... } from './types'` to a single
line now that only two symbols remain, which is what `lint:misc:check`
expects.
`buildMethodSignature` was reading the parameter name via `getNameNode().getText()`, which drops the leading `...` token on rest parameters. A handler defined as `doStuff(...args: string[])` rendered as `(args: string[]) => void` — parsed as a single array argument rather than varargs. Branch on `param.isRestParameter()` so the spread survives. Regression test added. `site/tsconfig.json` extends `../../../tsconfig.base.json`, which only resolves from the source location. `setupSite`'s `fs.cp` copied this file into the doc-generation output directory (and into published consumer installs), where the relative path resolves outside the project to a non-existent file. Docusaurus uses `jiti` and doesn't consult the tsconfig at runtime, so the build kept working with a silently broken file in the output. Add `tsconfig.json` to the skip set so the source file keeps inheriting from base (for IDE / lint typecheck inheritance, per #8012 r3277017046) without shipping a broken copy.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
❌ 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 7bb1849. Configure here.
| hooks: { | ||
| onBrokenMarkdownLinks: 'warn', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Docusaurus config nests onBrokenMarkdownLinks under invalid hooks key
Low Severity
The onBrokenMarkdownLinks setting is nested under markdown.hooks, but Docusaurus 3.6+ expects it directly at markdown.onBrokenMarkdownLinks — there is no hooks subobject in the Docusaurus markdown config schema. This means the setting is silently ignored. The default value happens to also be 'warn', so there's no behavioral difference today, but if anyone later changes this to 'throw' or 'ignore', the change would have no effect, which could be confusing.
Reviewed by Cursor Bugbot for commit 7bb1849. Configure here.
There was a problem hiding this comment.
This is a false positive — markdown.hooks.onBrokenMarkdownLinks is the canonical location in Docusaurus 3.10. From the installed @docusaurus/types@3.10.1:
node_modules/@docusaurus/types/src/markdown.d.ts:
export type MarkdownHooks = {
onBrokenMarkdownLinks: ReportingSeverity | OnBrokenMarkdownLinksFunction;
onBrokenMarkdownImages: ReportingSeverity | OnBrokenMarkdownImagesFunction;
};
export type MarkdownConfig = {
// ...
hooks: MarkdownHooks;
};node_modules/@docusaurus/types/src/config.d.ts:
// TODO Docusaurus v4 remove
onBrokenMarkdownLinks: ReportingSeverity | undefined;So the top-level config.onBrokenMarkdownLinks is the deprecated/legacy field (slated for removal in v4), and markdown.hooks.onBrokenMarkdownLinks is the forward-compatible nested form — which is exactly what our config uses. There's no markdown.onBrokenMarkdownLinks field at all. The setting is being read by Docusaurus, not silently ignored.
## 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? --> `resolveIndexedAccessMethod` still contains `istanbul ignore` comments. Most of these are unnecessary, and the logic actually ought to be tested. This commit cleans up these `istanbul ignore` comments, and also adds new ones to clarify the existing logic. It also renames `resolveIndexedAccessMethod` to `findClassMethodDeclaration` so that it's easier to understand the purpose of this function at a glance. ## 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 - [x] 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] > **Low Risk** > Documentation extraction only; behavior is clarified and covered by new tests, with no runtime product surface change. > > **Overview** > Refactors messenger **action handler** resolution in `platform-api-docs` extraction: **`resolveIndexedAccessMethod`** is renamed to **`findClassMethodDeclaration`**, with clearer guards, inline examples, and removal of **`istanbul ignore`** coverage skips on paths that are now meant to run in production. > > Adds **five tests** that assert when `Class['method']` cannot be resolved (object-literal type, `keyof`, numeric index, namespace-qualified class, missing method), docs still show the **raw handler type text** instead of failing or inventing a signature. Successful resolution behavior is unchanged; only naming, comments, and test coverage for fallbacks differ. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 561b1c6. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
## 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? --> It seems `extractFromFile` was only used in tests but nowhere else. `extractFromSourceFile` is the true public method. ## 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] > **Low Risk** > API removal only affects consumers of the removed helper; extraction logic is untouched and tests preserve the same scenarios. > > **Overview** > Removes the **`extractFromFile`** convenience API from `platform-api-docs` so callers use **`createExtractionProject`** plus **`extractFromSourceFile`** only—the path intended for real batch extraction. > > Tests are updated to match: a shared **`extractFromWrittenFile`** helper writes fixtures, loads sibling `.ts` / `.d.cts` files into a project, and runs **`extractFromSourceFile`**. The suite is renamed from **`extractFromFile`** to **`extractFromSourceFile`**; extraction behavior is unchanged. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit edd9bbd. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…onstructor (#8938) ## 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? --> In `tryToExtractFromCapabilityTypeConstructor`, we look up the name of the action/event type that is being declared, and we use a type guard to ensure that it is an `Identifier` before proceeding. But we already know that, because we determined it in `recursivelyFindMessengerCapabilityTypeDeclarations`. Currently we are using `istanbul-ignore` so we don't have to test the redundant check. But a better fix would be to modify `recursivelyFindMessengerCapabilityTypeDeclarations` so that it returns the `typeName` that it's encountered. This way we can use it directly without having to look it up, and we can remove the redundant check along with the `istanbul-ignore`. ## 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 - [x] 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] > **Low Risk** > Docs extraction-only refactor with added test coverage; behavior for qualified constructor aliases remains skip-without-crash. > > **Overview** > **platform-api-docs** messenger extraction now records the constructor utility name (`ControllerGetStateAction` / `ControllerStateChangeEvent`) when walking type aliases, but only when the alias body is a **plain identifier** reference—not a namespace-qualified name like `Helpers.ControllerGetStateAction<...>`. > > That lets `tryToExtractFromCapabilityTypeConstructor` use the pre-validated `typeName` instead of re-reading `body.getTypeName()` and dropping an `istanbul-ignore` for unreachable qualified-name handling. Qualified constructor aliases are skipped during the walk (same behavior as before, without redundant checks). > > A regression test covers a messenger union with both a local inline action and a qualified constructor alias: only the local action is documented. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit fd8e9d7. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…8939) ## 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? --> We're doing the same thing twice — we can collapse a conditional. Also look through the examples and fix any accuracy issues. ## 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] > **Low Risk** > Refactor-only change in docs extraction AST walking; logic is consolidated, not altered, and risk is low unless tests were not run. > > **Overview** > In **`packages/platform-api-docs/src/extraction.ts`**, **`recursivelyFindMessengerCapabilityTypeDeclarations`** no longer uses two separate blocks that both recurse into a type-alias body. **Union** bodies and **plain type references** (no type arguments) are handled by one shared condition and the same recursive walk, so umbrella aliases like `*MethodActions` and multi-action unions follow one code path. > > Comment-only updates clarify which `Actions` / alias shapes are valid, add examples for single-action re-exports and constructor references, and fix caret alignment in the template-literal `type` example in **`getMessengerCapabilityTypeString`**. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 3395e5c. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
mcmire
left a comment
mcmire
left a comment
There was a problem hiding this comment.
A few more things and then we should be good to go!
| 'merged-packages/**', | ||
| '.yarn/**', | ||
| 'scripts/create-package/package-template/**', | ||
| 'packages/platform-api-docs/site/**', |
There was a problem hiding this comment.
Hmm, why are we asking ESLint to ignore this directory? It seems like packages/platform-api-docs/site/docusaurus.config.ts would not get linted, which we probably don't want.
| 'packages/platform-api-docs/site/**', |
There was a problem hiding this comment.
I was getting OOM with eslint
| }, | ||
| }, | ||
| { | ||
| files: ['packages/platform-api-docs/**/*.{js,ts}'], |
There was a problem hiding this comment.
💡 I'm starting to wonder if we should have per-package ESLint files. Or if we should consolidate this config. It's getting a bit messy. Oh well, a problem for another day...
| }, | ||
| "resolutions": { | ||
| "elliptic@6.5.4": "^6.5.7", | ||
| "eslint-import-resolver-typescript": "3.7.0", |
There was a problem hiding this comment.
Why do we need these resolutions?
There was a problem hiding this comment.
I did forget why I had to; let me revert and check again
There was a problem hiding this comment.
These four pins were carried over from #8014 — they're version locks for the root's lint tooling, not transitive-dep conflict resolutions. yarn why eslint-import-resolver-typescript / eslint-plugin-import-x / eslint-plugin-n / prettier confirms each is consumed only by @metamask/core-monorepo itself.
I tried dropping them locally to see whether they were stale. With the pins removed, the floating versions become:
eslint-import-resolver-typescript: 3.7.0 → 3.10.1eslint-plugin-import-x: 4.6.1 → 4.16.2eslint-plugin-n: 17.15.1 → 17.24.0prettier: 3.4.2 → 3.8.3
yarn lint:eslint then fails because import-x/no-named-as-default (tightened somewhere after 4.6.x) flags pre-existing code in chain-agnostic-permission/src/scope/constants.ts:1 (Using exported name 'MetaMaskOpenRPCDocument' as identifier for default export), and one other location. So at least the eslint-plugin-import-x pin is load-bearing today; the other three are presumably the same kind of "keep lint deterministic" lock.
That said, none of this is needed for this PR — these pins predate the docs work and could be dropped (or just the now-stale ones) as a separate cleanup once the new lint errors are addressed. Want me to open a follow-up issue for that, or leave them as-is for now?
2 participants
Explanation
Adds
@metamask/platform-api-docs, a publishable package that generates and serves a searchable site documenting the Platform API — the catalog of actions and events available to clients through the message bus.The package scans TypeScript source and
.d.ctsdeclaration files, finds every*Messengertype declaration, walks itsActionsandEventstype arguments to discover the capability types they reference, extracts JSDoc/handler/payload information for each, and renders the result as a Docusaurus site with per-namespace pages and local search.Features
*Messengertype aliases and walksMessenger<Namespace, Actions, Events>to find the capability types — eliminating false positives from unrelated types that happen to share an action/event-like shape.type FooAction = { type: '...'; handler: ... }) and capability-type constructors (type FooGetStateAction = ControllerGetStateAction<typeof name, State>), including theirinterfacevariants.FooActions = FooGetAction | FooSetAction, the walker descends through it without documenting the intermediate alias.--scan-dirs,packages/*/src/(.ts), andnode_modules/@metamask/*/dist/(.d.cts).git remote get-url originandgit symbolic-ref refs/remotes/origin/HEADto buildhttps://github.com/<owner>/<repo>/blob/<branch>/<path>#L<line>URLs; falls back tomainfor shallow clones.--project-label Core/--project-label Extensionproduces titles likePlatform API (Core), and the short Git commit is shown in the intro and navbar so engineers can tell how current the docs are.Usage
From the core monorepo:
From client projects (Extension, Mobile), install
@metamask/platform-api-docsas a devDependency and add a script:{ "scripts": { "docs:platform-api:build": "platform-api-docs --build --project-label MyProject" } }Implementation
packages/platform-api-docs/— separate workspace from@metamask/messenger-cli(different deps and release cadence).execa. Flags:--build,--serve,--dev,--scan-dir(additive, repeatable),--output,--project-label. Configuration is CLI-only — nopackage.jsonconfig block.jsDoc.getDescription()+getTags()) replaces the previous hand-rolled comment parser.TypeAliasDeclaration/InterfaceDeclarationnodes directly tagged with'action'/'event'kind, so the main loop doesn't re-walk the source file looking up names.globpackage, with results sorted for deterministic output across filesystems.// istanbul ignorecomments.deploy-platform-api-docs.yml) builds docs on PRs (uploads the build as an artifact) and deploys to GitHub Pages onmain.References
Checklist
Note
Low Risk
Documentation tooling and CI only; no runtime controller or client API behavior changes.
Overview
Introduces
@metamask/platform-api-docs, a new workspace package that scans TypeScript (and@metamask.d.cts) for*Messengertypes, walks their Actions/Events unions, extracts JSDoc and handler/payload shapes, and emits a Docusaurus site with per-namespace pages, deduplication, and optional GitHub source links.The monorepo gains root scripts
docs:platform-api:build,dev, andserve, plusdeploy-platform-api-docs.ymlto build on PRs and publish to GitHub Pages under/platform-api/withkeep_files. ESLint ignores generated.platform-api-docs/and relaxes Node rules for the package; rootlint:eslintheap is raised and Yarn resolutions pin a few ESLint-related packages for the new Docusaurus dependency tree.CODEOWNERS, README package lists,
.gitignore, and Lavamoat allowlists are updated for the new package.Reviewed by Cursor Bugbot for commit 1675c03. Bugbot is set up for automated code reviews on this repo. Configure here.