-
-
Notifications
You must be signed in to change notification settings - Fork 18
types: create jsdoc based typescript typings #2
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
types: create jsdoc based typescript typings #2
Conversation
* @property {*} [Fragment] | ||
* @property {*} [jsxs] | ||
* @property {*} [recastPlugins] | ||
* @property {*} [useMDXComponents] |
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.
more definition around what these could/should be could be helpfup
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 guess there are no types for Fragment
, jsx
(which is missing here), and jsxs
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/a05cc538a42243c632f054e42eab483ebf1560ab/types/react/index.d.ts, perhaps because they are normally injected instead of supposed to be imported
* @property {*} [Fragment] | ||
* @property {*} [jsxs] | ||
* @property {*} [recastPlugins] | ||
* @property {*} [useMDXComponents] |
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 guess there are no types for Fragment
, jsx
(which is missing here), and jsxs
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/a05cc538a42243c632f054e42eab483ebf1560ab/types/react/index.d.ts, perhaps because they are normally injected instead of supposed to be imported
lib/evaluate.js
Outdated
* | ||
* @typedef {ProcessorOptions & RunnerOptions} ProcessorAndRunnerOptions | ||
* @typedef {{ [name: string]: any }} ComponentMap | ||
* @typedef {{ [props: string]: any, components?: ComponentMap }} XDMProps |
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.
Maybe MDXContentProps
instead? To me that explains more that they’re for the content component
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.
Maybe MDXContent
can also be defined as a function, which always returns a call to jsx
/ jsxs
/ pragma
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.
applied the first, can you expand what you mean by the second?
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.
The result of evaluate()
at .default
is a component: a function that takes MDXContentProps
and returns a call to jsx
: in React, I’m sure there’s a type for that.
Now, assuming jsx
had types in React, then there’d be a type for its return value, right? Then I’m guessing that you could take that value and define it as .default
s return value too?
Illustrated in another way: jsx
is a function: f() = x
. So the return value of .default
is also always x
(and for my comment I forgot that pragma
is not an option for evaluate
, and the return value of jsxs
and jsx
are the same)
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.
The result of evaluate() at .default is a component: a function that takes MDXContentProps
Right, and that is the ExportMap
type on line 19
Now, assuming jsx had types in React, then there’d be a type for its return value, right?
For react yes,
one that is consistent across React, Preact, and Vue, I'm not so sure.
I think there are differences (it would take some experimentation to check), and if there are differences, it would be difficult to capture even with conditional types watching what was passed to pragma
.
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 would be difference but the result of that f() = x
should be accessible though?
If someone does evaluateSync({jsx: f}).default()
the output type is still x
, and jsx
is a required option.
I understand that all the things passed as jsx
will be different, but they’re required to be functions, so that might help?
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.
Preact’s runtime does have types: https://github.com/preactjs/preact/blob/9250ac91891d19ec988938971b6767697bcb028e/jsx-runtime/src/index.d.ts#L11-L17
Co-authored-by: Titus <tituswormer@gmail.com>
Co-authored-by: Titus <tituswormer@gmail.com>
One extra thing is that TS is probably not reading |
I can add jsdocs to
adding cjs extension support in typescript is an open item at microsoft/TypeScript#38784 |
|
No description provided.