Skip to content
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

Support @deprecated jsdoc tag #20154

Merged
merged 3 commits into from
Jan 18, 2023
Merged

Support @deprecated jsdoc tag #20154

merged 3 commits into from
Jan 18, 2023

Conversation

yuisato1025
Copy link
Contributor

@yuisato1025 yuisato1025 commented Dec 8, 2022

Issue: #12759

What I did

  • Add deprecated property to ExtractedJsDoc type + test

How to test

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

@ndelangen
Copy link
Member

@yuisato1025 is this good to merge?

@ndelangen ndelangen marked this pull request as ready for review January 18, 2023 07:55
@ndelangen ndelangen merged commit 8285f7b into storybookjs:next Jan 18, 2023
@shilman
Copy link
Member

shilman commented Jan 18, 2023

@JReinhold @tmeasday can you please review this? It's already been merged but I'd love to get more eyes on it

Copy link
Contributor

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't figure out how to test this out.

We have a bunch of these @deprecated notations in the Description block, but they don't show up in the ArgsTable.

I've tried to trace this, and I can see the deprecated tags getting added all the way through to the React renderer's extractArgTypes. But when I log them in the ArgsJsDoc component in ArgsTable, they aren't there.

It looks like we're ignoring extractArgTypes for TS files in the react-vite framework (even though we are running it, because my logs are there), but it's a bit hard to tell, so that might not be the issue at all.

function extractDeprecated(tag: doctrine.Tag): ExtractedJsDocDeprecated {
if (tag.title != null) {
return {
name: tag.description,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would think this is a "description" rather than a "name"? That's at least how we use it in our TS prop types, and what I can gather from JSDoc specs:

  /**
   * @deprecated Manually specifying description type is deprecated. See https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#description-block-parametersnotes-and-parametersinfo
   */
  type?: DescriptionType;

@@ -81,6 +88,11 @@ export const ArgJsDoc: FC<ArgJsDocArgs> = ({ tags }) => {
<tr key={x.name}>
<td>
<code>{x.name}</code>
{hasDisplayableDeprecated && (
<div>
<span>Deprecated</span> {tags.deprecated}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given the existing implementation, shouldn't this access tags.deprecated.name and not tags.deprecated? That's what I gather from console logging a bunch of places at least.

Comment on lines +9 to +11
export interface JsDocParamDeprecated {
deprecated?: string;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar concern as above.

@ndelangen
Copy link
Member

@yuisato1025 Can you assist with resolving the issues @JReinhold has raised?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants