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.

Sign up for GitHub

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

Jump to bottom

fix typing on useArgs #18122

Closed
wants to merge 6 commits into from
Closed

fix typing on useArgs #18122

wants to merge 6 commits into from

Conversation

annacc-everlaw
Copy link
Contributor

Issue: useArgs is typed incorrectly.

What I did

Fixed useArgs to take a type parameter, P, which should be the type of the args for your story. By default, it's set to Args so that it's backwards compatible. Additionally, fixed the resetArgs function to be a (string[]) => void rather than a ([string]) => void, as a [string] makes no sense in this context (and is incorrect).

How to test

This is just a type change, so this can be tested by ensuring that things are typed correctly in your story when using useArgs<YourStoryArgType>.

  • 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.

@nx-cloud
Copy link

nx-cloud bot commented May 2, 2022 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 787fd9b. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

MichaelArestad
MichaelArestad approved these changes May 9, 2022
View reviewed changes
Copy link
Contributor

@MichaelArestad MichaelArestad left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you @annacc-everlaw

MichaelArestad
MichaelArestad requested changes May 9, 2022
View reviewed changes
Copy link
Contributor

@MichaelArestad MichaelArestad left a comment

Choose a reason for hiding this comment

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

Oop! It looks like the build is failing.

image

@annacc-everlaw
Copy link
Contributor Author

Ack, OK I'll get it building. Thanks for looking at it!

@annacc-everlaw
Copy link
Contributor Author

@MichaelArestad Heya Michael, sorry for the delay, just pushed a commit addressing the issue. Thanks again for looking at this!

@annacc-everlaw
Copy link
Contributor Author

One more change, let me know if it's ok!

@yannbf yannbf self-assigned this May 23, 2022
@annacc-everlaw
Copy link
Contributor Author

Bump!

@annacc-everlaw
Copy link
Contributor Author

Hey @MichaelArestad , not sure if there's anything more you need from me here! Thanks.

@annacc-everlaw
Copy link
Contributor Author

Don't know what I need to do here, if anything. Any guidance would be appreciated! Thanks

@MichaelArestad
Copy link
Contributor

Hey @annacc-everlaw. I will have someone take a look at this at the next weekly triage (Monday). Sorry for the delay!

@MichaelArestad MichaelArestad assigned shilman and unassigned yannbf Jun 27, 2022
@yannbf yannbf mentioned this pull request Jul 18, 2022
Closed
shilman
shilman reviewed Sep 7, 2022
View reviewed changes
lib/addons/src/hooks.ts Outdated Show resolved Hide resolved
@shilman shilman assigned kasperpeulen and unassigned shilman Sep 7, 2022
@ndelangen @shilman
Update lib/addons/src/hooks.ts
fbba1b9 
Co-authored-by: Michael Shilman <[email protected]>
@ndelangen
Copy link
Member

GItHub won't let me update to this PR, so I'll re-create it.

@ndelangen ndelangen closed this Jan 13, 2023
@ndelangen ndelangen mentioned this pull request Jan 13, 2023
Merged
@annacc-everlaw
Copy link
Contributor Author

Weird! Thanks for getting it merged! : )

@annacc-everlaw annacc-everlaw deleted the fix-use-args-types branch January 13, 2023 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shilman shilman shilman left review comments

@MichaelArestad MichaelArestad MichaelArestad requested changes

Assignees

@kasperpeulen kasperpeulen

Labels
core feature request typescript
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@annacc-everlaw @MichaelArestad @ndelangen @shilman @kasperpeulen @yannbf