-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add notes on how to type props and components #2510
Conversation
Signed-off-by: Karl Horky <[email protected]>
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Signed-off-by: Karl Horky <[email protected]>
Signed-off-by: Karl Horky <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2510 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 23 23
Lines 2693 2712 +19
Branches 2 2
=========================================
+ Hits 2693 2712 +19 ☔ View full report in Codecov by Sentry. |
This isn’t an MDX thing. It’s something Remco came up with for VS Code users (with which I am not saying it’s bad; it’s just, an idea, there). There are no tools to type check MDX files. TypeScript does not really have plugins yet. I don‘t think we’ve had a discussion about whether this should exist and how. I feel like we’d have to discuss many many things before we add this on the MDX website. Especially in this guide. Getting started is where MDX analyzer and TypeScript are covered. But even then: why should it be in the TS section when this isn’t a TS thing? And why should it be in the VS Code section when it’s already in the readme of our VS code integration? |
Interesting, I would say that the MDX Analyzer Also, maybe I'm misunderstanding "This isn't an MDX thing", but in my perception, an official "MDX" VS Code extension published by the maintainers of MDX does appear to be an MDX thing: I would personally see all UX/DX surface area here also as a part of the MDX experience.
Alright, understood. Happy to be a part of that discussion or not. My first thoughts, why I put it here:
Visual spaceIf it's undesirable for this to take up as much visual space, it could be collapsible as the other TS section is: Kapture.2024-07-08.at.14.07.35.mp4 |
There’s a difference with when you put something on the MDX website for how everybody can use the MDX format with all MDX tools, versus when you put something in a readme for
It’s a bit of a high-level statement, so also one from me: I generally don‘t think the proprietary closed-governance language TypeScript needs to be mixed into Javascript.
We can indeed mention how to type check props in the place where props are discussed. I agree that that is good. Lines 75 to 82 in 716ab3c
mdx-analyzer , seems more reasonable?
Some quick pseudo-text: <Note type="info">
**Note**:
Users of `vscode-mdx` can add type checking of `props` with a JSDoc comment.
See [`mdx-js/mdx-analyzer`][mdx-analyzer] for more info.
</Note>
…
<Note type="info">
**Note**:
Users of `vscode-mdx` can add type checking of provided and passed components
with a JSDoc comment.
See [`mdx-js/mdx-analyzer`][mdx-analyzer] for more info.
</Note>
…
[mdx-analyzer]: https://github.com/mdx-js/mdx-analyzer#use
Sure, but a) the robustness you want (and we’d also like to have!) doesn’t exist in MDX-the-language and most MDX tools yet, only in
Right, I had similar thoughts. I opted for a |
Signed-off-by: Karl Horky <[email protected]>
That makes sense to me 👍 Especially if it is not yet a decided syntax
Interesting idea - should I open a small strawman issue for this?
Happy to make some small edits here too, with a few words of guidance as to what you would expect
Yep, sounds like a good compromise + start 👍 I applied a version of the changes in: My single edit: added the link to the MDX VS Code extension page |
Signed-off-by: Karl Horky <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite good.
- one thing I wonder: should it go at the top or bottom? I was leaning to top. At the bottom it attaches to the next heading. Do you heave a reason for the tail end?
- I do think the 2 links are confusing. This isn’t Wikipedia. We don’t need to link every concept ever. Adding a link means people will open it and read it. You loose readers that way. This note is for people that already use the VS Code extension. Or, if not, and this piques their interest, the
mdx-analyzer
link leads them there. These links provide nothing and cost too much.
I think you as a user—especially with a background as a teacher—have great knowledge of what users and beginners might be missing. The problem space. So, I’d really appreciate it if you can post the problems you are experiencing. But I think the more solution space stuff is best answered by the maintainers. |
Signed-off-by: Karl Horky <[email protected]>
I chose / interpreted that you meant bottom because:
But I can move to top if preferred.
I tend towards over-linking rather than under-linking, and I think definitely for me as a user, I would want a link to know how to extend my experience. But I removed the link now: |
Signed-off-by: Karl Horky <[email protected]>
Props
Type section to Using MDX
thanks! |
Glad to help, thanks for the review and merge :) |
Initial checklist
Description of changes
Add a section on the Using MDX docs page to show how to type the
Props
object in TypeScript with JSDoc, and the features this unlocks with MDX Analyzer.I was unsure where to put visual assets, so I uploaded them to this PR description for now: