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. |
Source links generated by messenger-docs hardcoded `blob/main/` in the GitHub URL. For consumers whose default branch is something other than `main` (e.g. `master`, `develop`), every source link would 404. Resolve the default branch via `git symbolic-ref refs/remotes/origin/HEAD` when available, falling back to `main` if the symbolic ref isn't set (e.g. in shallow CI clones).
|
@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. |
Three targeted changes: - Remove the unreachable `TemplateExpression` branch from `resolveTypeString` in extraction.ts. The function is only called with `LiteralTypeNode.getLiteral()`, whose result is never a `TemplateExpression` — substituted template literals in type position are `TemplateLiteralTypeNode` and handled separately. - Add generate tests that initialize a real git repo in the sandbox (`git init` + `git remote add origin ...` + `git symbolic-ref`) to cover `resolveDefaultBranch` and the GitHub-vs-non-GitHub branches of `resolveRepoBaseUrl`. - Add markdown tests that exercise namespace pages containing both actions and events, including a `repoBaseUrl` source link and a deprecated item. Coverage moves from 86.72 / 70.88 / 94.54 / 86.92 (the temporarily lowered threshold) back to 91.35 / 75.39 / 94.64 / 91.59, restoring the original 91 / 72 / 92 / 91 threshold.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Adds two pieces of provenance to the generated site so engineers can tell at a glance which project the docs were generated from and how current they are: - The project label is resolved from `package.json#messenger-docs.title` when set, otherwise derived from `package.json#name` (`@metamask/core-monorepo` → "Core", `metamask-extension` → "Extension"). - The short Git commit SHA is read via `git rev-parse --short HEAD`. Both are plumbed into `generate()` (and into Docusaurus via the `DOCS_PROJECT_LABEL` / `DOCS_COMMIT_SHA` env vars) so that: - The browser title becomes `Platform API (Core) Reference | Platform API (Core)`. - The index page heading becomes `# Platform API (Core)` with a "_Generated from commit `<sha>`._" line in the intro. - A `commit <sha>` link appears in the navbar. Also fixes the `--scan-dir` flag to actually be additive, matching its documented "Additional directories" semantics. Previously, supplying `--scan-dir app` silently dropped the default `src` directory; the flag now appends to the base set (the `scanDirs` config, or `['src']`), deduplicating while preserving order.
|
@metamaskbot publish-preview |
1 similar comment
|
@metamaskbot publish-preview |
|
@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. |
There was a problem hiding this comment.
Did another pass on this, mainly with the goal of simplifying code. I know I have added some things we didn't talk about before but it seems it would be better to get this first PR as right as possible. It's much easier to add things later than remove them. Curious to know your thoughts.
| @@ -0,0 +1,60 @@ | |||
| name: Messenger API Docs | |||
There was a problem hiding this comment.
What do you think about calling this "Deploy Platform API Docs" and calling this file deploy-platform-api-docs.yml? Just want to make sure we stick with this terminology as much as we can.
There was a problem hiding this comment.
| steps: | ||
| - name: Deploy to GitHub Pages | ||
| id: deployment | ||
| uses: actions/deploy-pages@v4 |
There was a problem hiding this comment.
IIRC this action deploys to the root of the gh-pages branch and doesn't let you choose a subdirectory.
If we ever want to publish package API docs as opposed to platform API docs, it might be better if we put these somewhere specific.
What are your thoughts on using the peaceiris/actions-gh-pages action that we use in other places and deploying the site to a /platform-api subdirectory?
There was a problem hiding this comment.
Done in 34c820f.
- Swapped
actions/deploy-pagesforpeaceiris/actions-gh-pages@v4.1.0withdestination_dir: platform-apiandkeep_files: true— the deploy now publishes togh-pages/platform-api/and won't clobber any sibling subdirectories (e.g. a future/package-api/). - Updated
DOCS_BASE_URLto/<repo>/platform-api/so the built Docusaurus site has the right<base href>for serving from a subdirectory. - Replaced
actions/upload-pages-artifactwith a regularupload-artifact+download-artifacthandoff between the build and deploy jobs. - Permissions:
pages: write+id-token: write→contents: write(peaceiris pushes the build commit directly togh-pages). - Dropped the
environment: github-pagesblock since we're no longer using the Pages deployment environment. - Also bumped pinned action versions to match the freshest in this repo (
MetaMask/action-checkout-and-setup@v3,actions/upload-artifact@v6,actions/download-artifact@v7).
One thing to flag for when this merges: the repo's Pages settings need to be flipped from "GitHub Actions" back to "Deploy from a branch" (gh-pages branch), since we're no longer using the Pages Actions pipeline.
| * @param dir - The directory to search. | ||
| * @returns A promise that resolves to an array of absolute file paths. | ||
| */ | ||
| export async function findTsFiles(dir: string): Promise<string[]> { |
There was a problem hiding this comment.
Was 82b3e45 applied? I'm not seeing use of glob here.
There was a problem hiding this comment.
Good catch — that commit got reverted when messenger-docs was split out of messenger-cli, and I hadn't re-applied it. Re-applied in 9d2fbf7: findTsFiles and findDtsFiles are now thin wrappers around glob. One small addition — I explicitly sort the results so file processing order is deterministic across filesystems, since downstream dedup and output ordering depend on it.
| if (typeArgs.length >= 3) { | ||
| actionsArg = typeArgs[1]; | ||
| eventsArg = typeArgs[2]; | ||
| } else if (typeArgs.length === 2) { |
There was a problem hiding this comment.
It looks like we are also handling *Messenger<Actions, Events> types. Are there examples of this that you've found? All messenger types within core, snaps, or accounts should have three type arguments. Was this designed to handle messenger types prior to the introduction of @metamask/messenger?
There was a problem hiding this comment.
You're right — no real-world examples in core, snaps, or accounts. The 2-arg branch was leftover from a pre-@metamask/messenger shape and unused in practice. Dropped it in 36158b5 so the walker only accepts the 3-arg Messenger<Namespace, Actions, Events> form.
| if (visited.has(name)) { | ||
| return; | ||
| } | ||
| visited.add(name); |
There was a problem hiding this comment.
It looks like walkTypeReferences mutates its input. Since walkTypeReferences is designed to be called recursively, what are your thoughts on returning the output, making walkTypeReferences a pure function?
There was a problem hiding this comment.
Agreed — refactored in 36158b5. The walker is now collectNonUnionTypeNames, returns its result instead of mutating a passed-in Set, and keeps the recursion inside via a local closure so the recursive call sites don't have to pass a shared accumulator around.
| * @param context - Shared extraction context. | ||
| * @returns The extracted item, or null if the shape doesn't match. | ||
| */ | ||
| function extractFromInlineShape( |
There was a problem hiding this comment.
Nit: I'm not sure I've heard the term "inline shape" before. I can guess what it means, but you don't usually find these two words together (it's a bit like saying "red number"). Maybe this could be called extractFromInlineMessengerCapabilityType?
| function extractFromInlineShape( | |
| function extractFromInlineMessengerCapabilityType( |
There was a problem hiding this comment.
Applied your suggestion in 89832b1 — function renamed to extractFromInlineMessengerCapabilityType, and the JSDoc updated to drop "inline shape" in favor of "inline messenger-capability-type" throughout.
| * @param context - Shared extraction context. | ||
| * @returns The extracted item, or null if the helper doesn't match. | ||
| */ | ||
| function extractFromGenericHelper( |
There was a problem hiding this comment.
I don't think we would call ControllerGetStateAction or ControllerStateChangeEvent "helpers". But I know what this is trying to say. We don't have a good name for these. Maybe "type constructors"?
| function extractFromGenericHelper( | |
| function extractFromCapabilityTypeConstructor( |
There was a problem hiding this comment.
Applied your suggestion in 89832b1 — function renamed to extractFromCapabilityTypeConstructor, variable helperName → constructorName, expectedHelper → expectedConstructor, and the JSDoc now calls them "capability-type constructors" with a parenthetical noting that "type constructor" is the closest term we've got for these.
|
|
||
| const raw = jsDocs[0].getText().trim(); | ||
|
|
||
| // Handle single-line JSDoc: /** Gets the current state. */ |
There was a problem hiding this comment.
I feel like there ought to be a way we can do this more simply.
I asked Claude and it looks like the jsDocs object here also has a getDescription method, which returns the part above all of the tags. So we shouldn't need to parse the raw JSDoc text. We still need to manually get the @deprecate tag and append to the string we're building here but we can do that through another method, getTags. (It looks like we already use this method in getDeprecated.)
There was a problem hiding this comment.
Done in 66c8d81. extractJsDocText now uses jsDoc.getDescription() for the body and jsDoc.getTags() to find every @deprecated tag (rendered as **Deprecated:** <comment> lines). All other tags are dropped — their information is already in the rendered handler/payload signature. The hand-rolled line-by-line parser is gone (~100 lines).
One existing test (handles skipped tag continuation lines) was asserting a quirk of the old parser — that blank-line-separated text after @param tags was reappearing in the description. ts-morph follows standard JSDoc semantics and treats that as continuation of the last tag, so I rewrote the test to assert the more useful property: the description survives and the @param lines don't.
| * @param repoBaseUrl - Optional GitHub blob base URL (e.g. "https://github.com/Owner/Repo/blob/sha/"). | ||
| * @returns The generated markdown string. | ||
| */ | ||
| export function generateItemMarkdown( |
There was a problem hiding this comment.
Do we have to convert the "capability data blob" we've extracted to Markdown? Can we feed Docusaurus JSON instead and then use that to render the page for the capability? I feel like distilling that data into Markdown at this stage discards information that would be useful at the rendering stage and doesn't allow us to control formatting as much as we could. If we can work out a way to use JSON there may be more things in extraction.ts that we can remove (such as the MDX stuff at the bottom of extractJsDocText).
| const depText = text.slice('@deprecated'.length).trim(); | ||
| text = depText ? `**Deprecated:** ${depText}` : ''; | ||
| } else if (text.startsWith('@')) { | ||
| // Strip other tags (@param, @returns, @see, @throws, etc.) |
There was a problem hiding this comment.
Hmm. Why are we stripping @param and @returns? These are the things that describe the action handler or event payload. Right now we seem to be showing a raw representation of the handler or payload type (e.g. () => import("/Users/elliot/code/metamask/core/packages/app-metadata-controller/src/AppMetadataController").AppMetadataControllerState) and I think if we were to show the documented arguments and return type that would be much more useful.
There was a problem hiding this comment.
Good point — fixed in 671e21a.
@param and @returns are now extracted as structured data instead of being stripped: ExtractedMessengerCapabilityType gets params: { name, description }[] and returns: string fields. The markdown renderer emits a **Parameters**: name/description table and a **Returns**: line above the existing signature block (now labeled **Handler signature**: so the layered sections read naturally). Backtick-reference linkification flows through param descriptions and the returns line.
When an action's handler resolves to a class method via Class['method'] and the type alias itself has no JSDoc, the action inherits the method's @param / @returns automatically (with the type alias winning if both are present). Events don't get a parameters table — they carry positional payload tuples, not named arguments.
One thing worth flagging: the rich @param JSDoc for most actions in core lives in the auto-generated *-method-action-types.ts files, which our walker treats as imported leaves rather than descending into them. So while the H1 plumbing is complete, the live site won't show parameter tables for most ApprovalController/etc. actions until the cross-file resolution piece (extraction.ts:101) lands. That's the bigger refactor mcmire and I had been discussing as a follow-up.
mcmire
left a comment
mcmire
left a comment
There was a problem hiding this comment.
Noticed a few more things.
| ); | ||
| const pkg = JSON.parse(pkgRaw) as Record<string, unknown>; | ||
| const config = | ||
| (pkg['messenger-docs'] as MessengerDocsConfig | undefined) ?? {}; |
There was a problem hiding this comment.
Do we need to keep this? Can we rely on command-line arguments for configuration for now?
There was a problem hiding this comment.
Dropped in c7cde64. The package.json#messenger-docs config block is gone. --scan-dir is now actually additive (appends to the default ['src'], deduplicated, preserving order) and the project label moved to a --project-label flag. CLI-only now — readProjectPackageJson and resolveProjectLabel helpers are gone too.
| } | ||
|
|
||
| /** | ||
| * Derive a human-readable project label from a `package.json` name. Handles |
There was a problem hiding this comment.
Maybe this can be a command-line option so that we don't need to derive this?
There was a problem hiding this comment.
Done in c7cde64. There's now a --project-label flag; if omitted, no label is stamped (the title just becomes Platform API). The derivation logic (deriveProjectLabel) is gone. The root docs:platform-api:{build,dev,serve} scripts pass --project-label Core explicitly.
| // An object that configures minimum threshold enforcement for coverage results | ||
| coverageThreshold: { | ||
| global: { | ||
| branches: 75, |
There was a problem hiding this comment.
Is it possible to bring these numbers up to 100%?
There was a problem hiding this comment.
Got us to 100% lines and 100% on discovery.ts / markdown.ts in a6585b0. Statements landed at 99.59% and branches at 93.45%. The remaining gap is in defensive guards that protect against AST shapes the TypeScript compiler doesn't produce in real messenger-shaped source files (qualified-name type references, computed method names, non-property-signature members in capability-type literals, etc.) — those are marked with // istanbul ignore comments and short explanatory notes so future readers know they're intentional rather than overlooked.
Along the way I added a bunch of real fixtures for previously-untested paths: .json imports, missing import targets, destructured constants, externally-declared action references, non-Messenger types sharing the *Messenger suffix, capability-type constructors with mismatched or insufficient args, kind-mismatch dedup, broken-symlink failure paths, events-only namespaces, alphabetical namespace ordering, regenerating into an existing output directory, and the git symbolic-ref shallow-clone fallback.
Per review feedback, swap a few generic names for terms that better describe the concept: - `MessengerItemDoc` → `ExtractedMessengerCapabilityType`. "Item" was generic; we extract documentation about a messenger capability — an action or event type. - `extractItem` → `extractFromMessengerCapabilityType`, matching the `extractFromFile` naming pattern. - `extractFromInlineShape` → `extractFromInlineMessengerCapabilityType`. "Inline shape" was unfamiliar phrasing. - `extractFromGenericHelper` → `extractFromCapabilityTypeConstructor`. We don't have a settled term for `ControllerGetStateAction` / `ControllerStateChangeEvent` but "type constructor" is closer than "generic helper". - `collectMessengerReferences` → `collectMessengerCapabilityTypeNames`. "References" suggested call sites; the function returns type names. - Returned fields `actions` / `events` → `actionTypeNames` / `eventTypeNames` to make it obvious they are names, not whole types. - Workflow: `messenger-docs.yml` → `deploy-platform-api-docs.yml`, workflow display name → "Deploy Platform API Docs", aligning with the broader "Platform API" terminology used elsewhere.
Three structural cleanups in extraction.ts: - The walker (formerly `walkTypeReferences`) is now `collectNonUnionTypeNames` and is pure: it returns its result instead of mutating a caller-passed `Set`. The recursion is kept inside via a local closure. - The JSDoc for the walker now spells out the worked example from the review thread — given `AccountsControllerActions | AllowedActions`, which names land in the result set and which are deliberately skipped. - Drop the 2-arg `Messenger<Actions, Events>` branch. Every messenger type in core, snaps, and accounts uses the 3-arg form `Messenger<Namespace, Actions, Events>`; the 2-arg branch was leftover from a pre-`@metamask/messenger` shape and unused in practice.
Previously `collectMessengerCapabilityTypeNames` returned two sets of *names*, and `extractFromFile` then re-walked every top-level statement in the source file to find the declarations matching those names. That's two passes over the same AST, and the second pass was effectively a name-to-declaration lookup that the first pass already had in hand. Replace it with `collectMessengerCapabilityTypeDeclarations`, which resolves each leaf type-name to its local `TypeAliasDeclaration` or `InterfaceDeclaration` as it walks, tagging each with its kind. Skipping the second walk shrinks `extractFromFile`'s main loop down to a single iteration over the returned declarations.
Per review feedback, simplify configuration so it lives only on the command line. There were three sources of truth — derivation from `package.json#name`, a `messenger-docs.title` override, and the `messenger-docs.scanDirs` config — for no clear win. - Drop `messenger-docs.title` and `deriveProjectLabel`. Add a `--project-label` flag instead; if absent, no label is stamped. - Drop `messenger-docs.scanDirs`. Source directories are now the default `src/` plus anything supplied via `--scan-dir` (deduplicated). - Drop the package.json-reading helper. - Update the README to describe the CLI-only surface area. - Wire the root `docs:messenger:*` scripts to pass `--project-label Core`.
`extractJsDocText` previously stripped comment delimiters, asterisks,
and tag continuation lines by hand — a state-machine line-by-line
parser. ts-morph already does all of that:
- `jsDoc.getDescription()` returns the body above the first tag with
delimiters and leading asterisks already removed,
- `jsDoc.getTags()` returns the tags as structured nodes, each with a
`getCommentText()` for the trailing comment.
Replace the manual parser with calls to those APIs:
- Use the description verbatim.
- For every `@deprecated` tag, append a `**Deprecated:** <comment>`
line (with the comment's newlines collapsed to spaces).
- Drop all other tags (`@param`, `@returns`, `@see`, …) — their
information is already present in the rendered handler/payload
signature.
- Run the result through one `escapeJsDocTextForMdx` helper that
resolves `{@link}` and escapes stray curly braces.
One existing test (`handles skipped tag continuation lines`) asserted
a quirk of the hand-rolled parser — that a blank-line-separated chunk
of text *after* `@param` tags reappeared in the description. Standard
JSDoc semantics (which ts-morph follows) treat that as continuation of
the last tag, so the test is rewritten to verify the more useful
property: the description survives, the `@param` lines are gone.
Branch threshold dropped temporarily by ~2 points; will be restored in
the coverage commit.
Reviewer asked whether the package can hit 100% coverage. Most of the
remaining uncovered code was either reachable-but-untested or genuinely
defensive:
- **Reachable but missing tests.** Added focused fixtures and tests:
- `.json` imports skipped without resolution attempts
- relative imports whose candidate paths don't exist
- destructured `const` declarations
- non-Messenger types that share the `*Messenger` name suffix
- Messenger types with fewer than three type arguments
- union members that are not type references
- capability-type-constructor aliases with unknown names
- capability-type-constructor aliases with insufficient args
- externally-declared (imported) action references
- shared capabilities referenced by two messengers
- capability types where `type` is declared after `handler`
- dedup paths that exercise replace-same-kind, replace-different-kind,
and skip-lower-score branches
- git origin/HEAD missing (the shallow-clone fallback)
- per-file failures bubbling up as warnings
- sidebars/index variants for events-only namespaces, deprecation
markers, plural rendering, and the no-repoBaseUrl default
- **Defensive guards.** Cases that protect against AST shapes the
TypeScript compiler doesn't actually produce in messenger-shaped
source files (qualified-name type references, computed method names,
non-property-signature members in capability type literals, etc.) are
marked with `// istanbul ignore` comments and explanatory notes.
- **Restructured the template-literal-type span loop.** The previous
three-way `TypeQuery / LiteralType / else` branch chain had two arms
that no real-world messenger types exercise. The loop is now a single
pass over `${typeof X}` substitutions with `// istanbul ignore`
comments on the bail-out paths, which is what the messenger types we
extract from actually use.
Coverage thresholds raised to the new baseline: 99% statements, 100%
lines, 93% branches, 96% functions.
Re-applies an earlier change that was lost during the messenger-docs package split. `findTsFiles` and `findDtsFiles` are now thin wrappers around `glob`, which replaces the hand-rolled recursive readdir walks. `glob` doesn't guarantee result order, so the matches are explicitly sorted before being returned. Downstream consumers (extraction, dedup, output ordering) depend on a stable file processing order, so this keeps generated output deterministic across filesystems.
The package was originally named for where it lived in the codebase
(alongside `messenger-action-types`), but its actual product is the
Platform API documentation site: the catalog of actions and events
available through the message bus. The site title, the workflow
("Deploy Platform API Docs"), and the README copy all already use
"Platform API"; the package and CLI name were the last lingering bits
of the old framing.
Renames in this commit:
- Directory: `packages/messenger-docs/` → `packages/platform-api-docs/`.
- npm name: `@metamask/messenger-docs` → `@metamask/platform-api-docs`.
- Binary: `messenger-docs` → `platform-api-docs` (now explicitly named
in `bin` rather than implicitly derived from the package name).
- Root scripts: `docs:messenger:{build,dev,serve}` → `docs:platform-api:*`.
- Output directory: `.messenger-docs/` → `.platform-api-docs/`.
- Generated `sidebars.ts` header comment and the synthetic Docusaurus
site's `package.json#name` updated to match.
- CODEOWNERS, teams.json, tsconfig.{json,build.json}, eslint.config.mjs,
yarn.config.cjs, .gitignore, and the workflow file all updated to
reference the new path/name.
The package is still at 0.0.0 with no consumers, so there's no
migration path needed.
The `node_modules/` branch in `generateItemMarkdown` was producing a nonsensical link when the path wasn't `@metamask/`-scoped: the fallback used the entire source path as the package name, yielding URLs like `https://www.npmjs.com/package/node_modules/some-vendor/dist/index.d.ts`. In practice the scanner only walks `node_modules/@metamask/*/dist`, so the fallback never fired against real input. The only thing exercising it was a unit test I added in the coverage-bump commit — so the test was effectively codifying broken output to inflate coverage. Restructure to: render an npm link only when the path is actually `@metamask`-scoped, otherwise fall through to the normal source-link branches (GitHub blob URL if a repo base URL is available, plain path otherwise). The two unit tests now assert the correct behavior: - a plain `**Source**: \`<path>:<line>\`` when no repo URL is given - a GitHub blob URL when one is Reported by Cursor Bugbot on PR #8012.
cryptodev-2s
changed the title
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ 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 1b1b547. Configure here.
`actions/deploy-pages` deploys to the root of the gh-pages site, which means a single repo can only publish one doc site through it. If we ever want to add package API docs (or any other doc surface) alongside, the platform-api site needs to live in its own subdirectory. Switch the deploy mechanism to `peaceiris/actions-gh-pages` and: - Publish to `gh-pages/platform-api/` via `destination_dir: platform-api`. - Set `keep_files: true` so sibling subdirectories (e.g. a future `/package-api/`) aren't clobbered on each platform-api deploy. - Update `DOCS_BASE_URL` to `/<repo>/platform-api/` so the built Docusaurus site has the right `<base href>`. - Replace the `actions/upload-pages-artifact` step with a regular `actions/upload-artifact` + `actions/download-artifact` pair, since we no longer use the Pages-specific artifact pipeline. - Swap the `pages: write` / `id-token: write` permissions for the deploy job to `contents: write` (peaceiris pushes the build commit directly to `gh-pages`). - Drop the `environment: github-pages` block; with peaceiris, the deploy isn't gated on a Pages deployment environment. - Bump pinned action versions to match the freshest in this repo: `MetaMask/action-checkout-and-setup@v3`, `actions/upload-artifact@v6`, `actions/download-artifact@v7`, `peaceiris/actions-gh-pages@v4.1.0`. Note: when this lands, the repo's GitHub Pages settings need to be flipped from "GitHub Actions" back to "Deploy from a branch" (gh-pages) since we're no longer using the Pages Actions pipeline.
`resolveHandler` was matching only the single-quoted form `ClassName['method']`. TypeScript accepts three syntactically distinct forms for indexed-access types — `Class['m']`, `Class["m"]`, and `` Class[`m`] `` — and ts-morph's `getText()` returns whatever the source used verbatim. When a controller author chose double quotes or template literals, we'd fail to resolve the handler and fall back to rendering the raw text instead of the method signature plus its JSDoc. Updated the regex to a backreferenced quote character class (`/^(\w+)\[(['"`])(\w+)\2\]$/u`) and shifted the method-name capture group from `match[2]` to `match[3]` to compensate. Added two tests covering the double-quoted and template-literal forms. Bump the action versions in the deploy workflow to the absolute latest pair (`upload-artifact@v7` / `download-artifact@v8`), since these are paired by major version and v8 of `download-artifact` flips hash-mismatch handling from warning to error (a strictly-better default for an artifact-driven deploy). Reported by Cursor Bugbot on PR #8012.
The site was rendering action handlers as raw TypeScript types — fine
when the types are clean, but ugly when ts-morph emits a fully-qualified
`import("/abs/path/...").TypeName`. The information engineers actually
want — what each handler argument means and what it returns — was
being thrown away (the `@param` / `@returns` tags were stripped by the
JSDoc parser before reaching the renderer).
Keep those tags as structured data and render them alongside the
handler signature:
- Add `params: { name, description }[]` and `returns: string` fields to
`ExtractedMessengerCapabilityType` (and `MethodInfo` for inheritance).
- Replace `extractJsDocText` with `extractJsDoc`, which decomposes the
comment into `{ description, params, returns }` — the description
body still flows through MDX-escaping and `{@link}` resolution, but
`@param` and `@returns` tags are pulled out into separate fields.
- Strip the conventional leading `- ` separator from `@param` comments
(JSDoc style is `@param name - description`; the hyphen is purely
decorative).
- When an action's handler resolves to a class method, inherit the
method's `@param`/`@returns` if the type alias doesn't have its own.
Type-alias-level docs win when both are present.
- In `markdown.ts`, render `**Parameters**:` as a name/description
table and a `**Returns**:` line, both with backtick-reference
linkification. The existing signature block is renamed to
`**Handler signature**:` / `**Payload signature**:` so the new sections
read naturally.
Events don't get a parameters table — they carry positional payload
tuples, not named arguments.
Coverage held at 100% lines / 94% branches with new fixtures for the
structured-fields path, multi-line params, and class-method inheritance
(including the type-alias-overrides-method ordering rule).
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
Medium Risk
Adds a new publishable docs generator plus a GitHub Pages deploy workflow and new tooling/dependency resolutions, which could affect CI/build behavior across the monorepo if misconfigured.
Overview
Adds new
@metamask/platform-api-docsworkspace that scans TS and.d.ctsfiles for*Messengertype declarations, extracts action/event docs (including JSDoc), and can--build/--dev/--servea Docusaurus site via a new CLI.Wires the package into the monorepo via new root
docs:platform-api:*scripts, TypeScript project references, eslint ignores/rules, team ownership/README listing, and ignores generated.platform-api-docs/output.Adds
deploy-platform-api-docs.ymlto build docs on PRs (artifact upload) and deploy/platform-api/to GitHub Pages onmain, and updates root dependencyresolutions/Lavamoat allowlist for the new tooling.Reviewed by Cursor Bugbot for commit 671e21a. Bugbot is set up for automated code reviews on this repo. Configure here.