feat(language-service): Make MDX nodes typed with JSX.IntrinsicElements#515
feat(language-service): Make MDX nodes typed with JSX.IntrinsicElements#515remcohaszing merged 4 commits intomdx-js:mainfrom
Conversation
🦋 Changeset detectedLatest commit: cc9043d The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This comment has been minimized.
This comment has been minimized.
f871eb3 to
f0dc9b8
Compare
remcohaszing
left a comment
There was a problem hiding this comment.
The code looks great, but the tests need to be adjusted, notably the tests in https://github.com/mdx-js/mdx-analyzer/blob/main/packages/language-service/test/language-plugin.js.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #515 +/- ##
==========================================
+ Coverage 92.31% 92.43% +0.11%
==========================================
Files 13 13
Lines 1927 1982 +55
==========================================
+ Hits 1779 1832 +53
- Misses 148 150 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Ok this looks great! Just one small thing left: add a changeset, so we make a release.
|
This comment was marked as resolved.
This comment was marked as resolved.
|
Thank you! |
Initial checklist
Description of changes
This PR tries to map MDX Markdown node to correct JSX node. The feature request is at https://github.com/orgs/mdx-js/discussions/2639 . Before, the MDX node are translated to
<></>fragment in language service, which is not typed well (see microsoft/TypeScript#62358) and this PR changes them to a slightly corrected translation, such as<p></p>.Benefit on this, we can do type-checking on JSX expression 'hole' inside Markdown contents, such as (e.g. translate to React JSX):
Also some open questions:
listItem { paragraph { innerContent } }for a compact list item, which is not the correct behavior according to CommonMark spec (says no<p>inside a<li>may not work;