-
Notifications
You must be signed in to change notification settings - Fork 9
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: types #877
fix: types #877
Conversation
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.
⌨️
(TIL there's no typewriter emoji???)
|
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.
looks like there's a type error on line 96 now?
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.
@@ -112,12 +112,12 @@ export const html = (text: string, opts = {}) => { | |||
unimplemented('html export'); | |||
}; | |||
|
|||
const astProcessor = (opts = {}) => | |||
const astProcessor = (opts: MdastOpts = { components: {} }) => |
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.
Can components
get more specific or is it just Record<string, string>
?
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.
I don't think so? It's a mapping of tag name to component body?
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.
might want to narrow it to Record<string, string>
because right now it'll allow Record<string, unknown>
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.
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.
oh my bad. MdastOpts
is where components
is defined, I misread the =
there as :
and thought you were typing opts: { components: {} }
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.
Thought I was about to have my type system shattered.
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.
nope im just a dumbass. carry on
This PR was released!🚀 Changes included in v6.75.0-beta.32 |
🧰 Changes
Fixes a bunch of types from the jsx-coercion PR.
🧬 QA & Testing