-
-
Notifications
You must be signed in to change notification settings - Fork 876
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
Fix TypeScript bug by adding workaround #676
Conversation
56cee9a
to
baf0ac6
Compare
This goes against the idea of using TS through JSDoc, to solve an issue where TypeScript does not support PnP. Related-to: syntax-tree/mdast-util-gfm#1 (comment). |
Any thoughts on my comment? I’m leaning on seeing this as an issue TS should solve. |
Would you be open to adding |
Where would that have to be added? Because the error line includes that undefined already. Also it sounds kinda weird that adding undefined somewhere "fixes" an issue, and that both behaviors are expected. |
It would be added here: - * @property {import('remark-rehype').Options} [remarkRehypeOptions={}]
+ * @property {import('remark-rehype').Options | undefined} [remarkRehypeOptions={}] |
Oh I’m up for an explicit |
This is required for TypeScript to avoid referencing the wrong type (see microsoft/TypeScript#48242 (comment)).
baf0ac6
to
fee8a7e
Compare
PR updated. I agree, feel free to add a comment to microsoft/TypeScript#48242. I haven't gotten a response to my last two comments. |
This comment has been minimized.
This comment has been minimized.
Thanks, released! https://github.com/remarkjs/react-markdown/releases/tag/8.0.2 |
Initial checklist
Description of changes
Follow-up to #675. Adds
| undefined
as suggested in microsoft/TypeScript#48242 (comment).Fixes this build error: