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

remove the defaultValue feature for argTypes #19492

Merged
merged 8 commits into from
Jan 14, 2023

Conversation

ndelangen
Copy link
Member

remove the deprecated defaultValue property api for argTypes

@ndelangen ndelangen requested review from tmeasday and shilman October 14, 2022 16:09
@ndelangen ndelangen self-assigned this Oct 14, 2022
@ndelangen ndelangen added the maintenance User-facing maintenance tasks label Oct 14, 2022
@ndelangen
Copy link
Member Author

@shilman I'd like your review on this.. I commented out the test code, because I didn't know if I should remove it or modify it, please advice what you'd like me to do about that.

I also suspect that there might be stories (in our own codebase) out there somewhere that use this deprecated feature?

Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

Yes, I think I discovered in the angular work that it is still setting default values from compodoc (which is often very annoying, TBH). Still it might make it a disruptive changes for angular users.

Probably we need to pull the bandaid though.

@ndelangen ndelangen assigned shilman and unassigned ndelangen Oct 17, 2022
@ndelangen
Copy link
Member Author

Discussed with @shilman he'll take a look at this, when he has time.

I'll continue with other deprecations removal work.

@ndelangen
Copy link
Member Author

This is one of the few remaining ones @tmeasday @shilman

@tmeasday
Copy link
Member

tmeasday commented Oct 21, 2022

@ndelangen I think you should play around with some of the angular stories with and without this change. For example this line of code:

https://github.com/storybookjs/storybook/blob/next/code/frameworks/angular/template/stories/argTypes/doc-button/doc-button.component.ts#L52

I'm pretty sure it leads to an argType.defaultValue being set for the story automatically to { theDefaultValue: 'Default value in component' }. I'm also pretty sure we changed the react docgen->argTypes implementation so that wouldn't happen (in the equivalent situation.

@ndelangen
Copy link
Member Author

@shilman Do you think it's possible for us to resolve this together on Monday? and get this merged?

@shilman
Copy link
Member

shilman commented Oct 23, 2022

@ndelangen Sure! Let's do it

@tmeasday
Copy link
Member

@ndelangen sorry for the delay here, I got to the bottom of it: #19935

@ndelangen ndelangen assigned tmeasday and unassigned shilman Nov 25, 2022
@ndelangen ndelangen merged commit e886dd4 into next Jan 14, 2023
@ndelangen ndelangen deleted the deprecate/argType-defaultValue branch January 14, 2023 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants