Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Nice! A few stylistic comments, but implementation wise is relatively good to go! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #668 +/- ##
==========================================
+ Coverage 75.90% 76.10% +0.19%
==========================================
Files 145 145
Lines 13718 13836 +118
Branches 991 1014 +23
==========================================
+ Hits 10413 10530 +117
- Misses 3299 3300 +1
Partials 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…, cleanup fallback)
There was a problem hiding this comment.
Pull request overview
Adds basic TypeScript generic-type support to the parser so {Promise<string>}-style annotations are converted into appropriate Markdown links (with escaped < / >), improving type reference rendering in generated docs.
Changes:
- Update
transformTypeToReferenceLinkto preserve< >initially, detect basic generics, and split outer unions safely. - Add unit tests covering recognized, partially recognized, union, multi-parameter, and outer-union generic types.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/utils/parser/index.mjs | Implements generic parsing/formatting in transformTypeToReferenceLink, plus a new union-splitting helper. |
| src/utils/parser/tests/index.test.mjs | Adds test coverage for basic generic parsing scenarios and expected escaped output. |
Comments suppressed due to low confidence (3)
src/utils/parser/index.mjs:186
splitByOuterUniondecrementsdepthon every>character. If the type string contains>outside of generics (e.g. arrow types=>),depthcan become negative and outer|unions will stop splitting, changing behavior vs the previoustypeInput.split('|')implementation. Consider only decrementing whendepth > 0(or clamping to 0) so non-generic>doesn't affect union parsing.
// Transform Node.js type/module references into Markdown links
// that refer to other API docs pages within the Node.js API docs
if (record && lookupPiece in record) {
return record[lookupPiece];
}
// Transform Node.js types like 'vm.Something'.
if (lookupPiece.indexOf('.') >= 0) {
const [mod, ...pieces] = lookupPiece.split('.');
src/utils/parser/index.mjs:156
replace('[]', '')removes the first[]occurrence anywhere in the string; for array types the code appears to want to strip only a trailing[]before lookup. Consider removing only an end suffix to avoid incorrect matches.
* Handles the mapping (if there's a match) of the input text
* into the reference type from the API docs
*
* @param {string} lookupPiece
src/utils/parser/index.mjs:211
- The comment says a regex is used to remove
[]from the end of the type, but the code usesplainPiece.replace('[]', ''), which is not end-anchored and only removes the first occurrence. Update the implementation (and/or the comment) so it accurately strips only a trailing array suffix.
const result = transformType(trimmedPiece.replace('[]', ''));
// If we have a valid result and the piece is not empty, we return the Markdown link
if (trimmedPiece.length && result.length) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
Thank you so much for the review and the kind words 🤍 I really appreciate your time and the stylistic feedback, |
Description
Currently, the parser doesn't properly handle basic TypeScript generics like
{Promise<string>}. This PR adds support for parsing these basic generic types withintransformTypeToReferenceLink.It safely separates the base type and inner type, mapping both to their respective Markdown links (e.g.,
[](...)<[](...)>).Validation
I added new unit tests in
src/utils/parser/__tests__/index.test.mjsto cover both fully recognized and partially recognized generic types.Reviewers can verify the logic by running
node --run test:coverage. All tests pass successfully without breaking the existing fallback parsing logic.(Note: The parsed output intentionally uses
<and>entities to avoid potential downstream MDX/HTML rendering errors).Related Issues
Fixes #666
Check List
node --run testand all tests passed.node --run format&node --run lint.