react-docgen-typescript
react-docgen-typescript copied to clipboard
feat: Export raw expression when shouldIncludeExpression is set
Resolves #432 cc @joshwooding
This PR adds a property called rawExpression
to the parsed object when shouldIncludeExpression
is set, in addition to the existing expression
property. The two are identical in many cases, but diverge when the exported component has a wrapper such as React.memo
. Example from the test cases:
// parsed.expression
function Jumbotron(props: JumbotronProps) {
return <div>Test</div>;
}
// parsed.rawExpression
export default React.memo(Jumbotron);
Exporting both will allow developers to extract additional information from the raw expression when necessary.
@pvasek Sorry to bother you, do you know if we can merge this and do a release?
I understand the problem you are trying to solve. I think it's a little bit harder to know if that's (#433) really a bug or not. I mean who knows how the expression
is really used? So I understand your solution will not break anyone's code (most probably 🙂)
The only thing that worries me is that we are expanding public API and what if we find there is a similar issue somewhere else and we would end up with something like rootRawEpxression
or something similar.
It would be great to hear someone who is using that expression
property already and what are their takeaways.
I understand the problem you are trying to solve. I think it's a little bit harder to know if that's (#433) really a bug or not. I mean who knows how the
expression
is really used? So I understand your solution will not break anyone's code (most probably 🙂)The only thing that worries me is that we are expanding public API and what if we find there is a similar issue somewhere else and we would end up with something like
rootRawEpxression
or something similar.It would be great to hear someone who is using that
expression
property already and what are their takeaways.
That's a good point. I don't really expect many people to be using the expression
property. I have an example of using the rawExpression property though: https://github.com/joshwooding/vite-plugin-react-docgen-typescript/pull/3/files#diff-18112469e0b5ce0660c8b3e9bbdf6d1baf6099883d45a75ea2f8ac502c90af3dR22 which in my opinion is what the expression property was trying to solve initially.
I don't really expect many people to be using the
expression
property. I have an example of using therawExpression
property though…
Hmm. That example uses rawExpression
to get the component identifier, but I think there are cases where expression
is actually better suited to that purpose, as seen in the test case here… right?
Since getting the component identifier is the main reason expression
was added, and also the main reason why rawExpression
was requested, the ideal solution here would probably be to export the component identifier directly, rather than pushing that work onto consumers like react-docgen-typescript-plugin
and vite-plugin-react-docgen-typescript
.
I see one notable use case for exporting the expression, though, and that's for the case where the component has no identifier (for example, export default function() { /* component code */ }
). In that case the downstream would need to modify the AST in order to add an identifier (e.g. transforming the above to export default function asd1234() { /* component code */ }
). For that particular example, expression
and rawExpression
are identical, so there may not be a need to export both? Though maybe there's a variant on the case that I'm not thinking of.
I don't really expect many people to be using the
expression
property. I have an example of using therawExpression
property though…Hmm. That example uses
rawExpression
to get the component identifier, but I think there are cases whereexpression
is actually better suited to that purpose, as seen in the test case here… right?Since getting the component identifier is the main reason
expression
was added, and also the main reason whyrawExpression
was requested, the ideal solution here would probably be to export the component identifier directly, rather than pushing that work onto consumers likereact-docgen-typescript-plugin
andvite-plugin-react-docgen-typescript
.I see one notable use case for exporting the expression, though, and that's for the case where the component has no identifier (for example,
export default function() { /* component code */ }
). In that case the downstream would need to modify the AST in order to add an identifier (e.g. transforming the above toexport default function asd1234() { /* component code */ }
). For that particular example,expression
andrawExpression
are identical, so there may not be a need to export both? Though maybe there's a variant on the case that I'm not thinking of.
expression in the test case equals FunctionComponent which is the type of the function and not really usable whilst the rawExpression is the default export. I guess returning the identifier is also a suitable solution too though.
Thank you for explaining these details to me.
If we really need both could we come up with a better name than rawExpression
? Something that would explain what to really expect there?
Thank you for explaining these details to me.
If we really need both could we come up with a better name than
rawExpression
? Something that would explain what to really expect there?
Do you think rawExpression
should be preferred over outputting the identifier?
Any update on this?
Thank you for explaining these details to me. If we really need both could we come up with a better name than
rawExpression
? Something that would explain what to really expect there?Do you think
rawExpression
should be preferred over outputting the identifier?
What should outputting the identifier looks like? How it will be different from rawExpression
?
If I understand it correctly we have two distinct options:
-
rawExpression
preferably with better name - outputting the identifier, whatever it means
Thank you for explaining these details to me. If we really need both could we come up with a better name than
rawExpression
? Something that would explain what to really expect there?Do you think
rawExpression
should be preferred over outputting the identifier?What should outputting the identifier looks like? How it will be different from
rawExpression
?If I understand it correctly we have two distinct options:
rawExpression
preferably with better name- outputting the identifier, whatever it means
The identifier would just be the variable or function name of the declaration. This can be computed using rawExpression.
- I guess something like
rootExpression
or even replacingexpression
would be fine to me. - This is the option that doesn't leak internals but is less flexible.
@TrevorBurnham What do you think about changing rawExpression
to rootExpression
?
@TrevorBurnham any update on this?
@pvasek @joshwooding Sorry for the delay in getting back to you. As suggested, I've changed rawExpression
to rootExpression
here. 👍
I do think this is something of a workaround rather than an ideal patch. To reiterate my comment above:
the ideal solution here would probably be to export the component identifier directly, rather than pushing that work onto consumers like
react-docgen-typescript-plugin
andvite-plugin-react-docgen-typescript
That would mean implementing an algorithm that can extract the identifier from components no matter how they're defined. So identifier
would be Rosebud
for all of these scenarios (just to name a few):
export const Rosebud = () => {};
export function Rosebud() {};
export class Rosebud extends React.Component {};
const Rosebud = () => {};
export default Rosebud;
export const Rosebud = hoc(Sled);
That way, packages like the one I mentioned could use the identifier
to generate code like Rosebud.displayName = 'Rosebud'
.