Skip to content

feat(type-parsing): use typescript#887

Open
avivkeller wants to merge 4 commits into
mainfrom
typescript
Open

feat(type-parsing): use typescript#887
avivkeller wants to merge 4 commits into
mainfrom
typescript

Conversation

@avivkeller

@avivkeller avivkeller commented Jul 8, 2026

Copy link
Copy Markdown
Member
Screenshot 2026-07-08 at 2 20 39 PM Screenshot 2026-07-08 at 2 21 14 PM

@vercel

vercel Bot commented Jul 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
api-docs-tooling Ready Ready Preview Jul 10, 2026 2:54am

Request Review

@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.36846% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.75%. Comparing base (d7bdf39) to head (046386f).

Files with missing lines Patch % Lines
src/utils/remark.mjs 62.06% 11 Missing ⚠️
src/generators/metadata/utils/resolveTypes.mjs 97.69% 5 Missing and 1 partial ⚠️
src/generators/man-page/utils/converter.mjs 25.00% 3 Missing ⚠️
src/generators/legacy-html/utils/buildContent.mjs 0.00% 1 Missing ⚠️
src/utils/queries/utils.mjs 50.00% 1 Missing ⚠️
src/utils/type-annotations/hast.mjs 99.11% 1 Missing ⚠️
src/utils/unist.mjs 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #887      +/-   ##
==========================================
+ Coverage   85.04%   85.75%   +0.70%     
==========================================
  Files         179      186       +7     
  Lines       16496    17162     +666     
  Branches     1500     1552      +52     
==========================================
+ Hits        14029    14717     +688     
+ Misses       2457     2436      -21     
+ Partials       10        9       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment on lines +43 to +47
current.textRaw = transformNodesToString(
child.children.filter(node => node !== subList)
)
.replace(/\s+/g, ' ')
.replace(/<!--.*?-->/gs, '');
// Shiki tokenizes `vm`, `.`, `Module` separately; the whole range links
const anchors = html.match(/href="vm.html#class-module"/g);
assert.ok(anchors.length >= 1);
assert.match(html.replace(/<[^>]+>/g, ''), /^vm\.Module$/);
@avivkeller avivkeller marked this pull request as ready for review July 8, 2026 21:20
@avivkeller avivkeller requested a review from a team as a code owner July 8, 2026 21:20
@cursor

cursor Bot commented Jul 8, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches core doc parsing and all output formats (metadata, legacy HTML/JSON, web UI); behavior changes for complex types and linking, though coverage is extensive.

Overview
Replaces the old regex-and-string rewriter for API {Type} references with a TypeScript-backed pipeline aligned to spec 1.0.1: brace spans are full TS type expressions, batch-parsed, and rendered as one inline code fragment with per-identifier links.

Parsing: A new Remark/micromark extension turns balanced {...} in non-MDX Markdown into typeAnnotation mdast nodes (nested braces, GFM escapes, table \|). Typed-list heuristics and legacy JSON/JSX list parsing now key off those nodes instead of <Type> link patterns.

Resolution: createTypeResolver() (lazy typescript import) attaches node.data.links via whole-value type-map keys, per-identifier lookup (built-ins, MDN, dotted vm.Module heuristic), with warnings and plain code on parse failure. Metadata parseApiDoc resolves annotations once per file; MDX still skips this pass.

Rendering: Rehype handlers emit <code class="type"> with ranged <a class="type-link"> anchors; the web pipeline adds Shiki TypeScript highlighting. Legacy HTML no longer post-processes pre-built type links. typeParser.mjs and text-based visitTextWithTypeNode are removed.

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

@avivkeller

Copy link
Copy Markdown
Member Author

The diff was too big to leave as a comment

Comment thread src/utils/type-annotations/hast.mjs

@bmuenzenmeyer bmuenzenmeyer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

a first pass

Comment thread docs/specification.md

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This changes the version of the spec right? How have we been handling this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This would be the first change to the spec, however, it's more of a change in wording that behavior. The only behavioral change is that {string}|{boolean} must now be {string|boolean}. Previously, the spec didn't explicitly say whether to do this or not to do this, now it explicitly says to keep all types with the {...} brackets.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

1.0.0 -> 1.1.0 sounds good?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i am less concerned if we think its still valid 1.0.0 due to omission - but we should develop some governance over what constitutes a change. imagine semver would work in moooooooost cases

Comment thread package.json
@avivkeller

Copy link
Copy Markdown
Member Author

@bmuenzenmeyer i ended up patching instead of minoring since we previously claim to support these types and similar ones, and this is more of an improvement on that claim than a new feature of the spec.

@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 using default effort and found 1 potential issue.

Fix All in Cursor

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

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 046386f. Configure here.

});
} else {
pending.push(node);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Whole-value lookup skips TypeScript

Medium Severity

Before batch TypeScript parsing, a non-empty lookupTypeName result treats the entire annotation as one link. That helper also hits built-in and MDN maps, not just the toolchain type map. Annotations that are not valid TypeScript but match those keys (e.g. display-style names in the built-in map) skip parsing, emit no invalid-annotation warning, and render as a single link instead of plain unlinked code per the spec.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 046386f. Configure here.

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.

4 participants