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: do not format callbacks with arguments as React hooks #5778

Merged
merged 5 commits into from
Jan 21, 2019
Merged

fix: do not format callbacks with arguments as React hooks #5778

merged 5 commits into from
Jan 21, 2019

Conversation

SimenB
Copy link
Contributor

@SimenB SimenB commented Jan 20, 2019

See discussion in #5608 (comment)

  • I’ve added tests to confirm my change works.
  • (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory)
  • I’ve read the contributing guidelines.

✨Try the playground for this PR✨

@SimenB SimenB mentioned this pull request Jan 20, 2019
Merged
2 tasks
j-f1
j-f1 approved these changes Jan 20, 2019
View reviewed changes
Copy link
Member

@j-f1 j-f1 left a comment

Choose a reason for hiding this comment

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

Want to do a 1.16.1 @ikatyang?

@ikatyang
Copy link
Member

We just added a CHANGELOG.unreleased.md in #5775, could you add a changelog item in that file?

(I plan to release 1.16.1 tonight (UTC+8) if there's no more issue reported.)

@ikatyang ikatyang added this to the 1.16.1 milestone Jan 21, 2019
@SimenB
Copy link
Contributor Author

SimenB commented Jan 21, 2019 •
edited
Loading

Sure, gave it a go!

I'm heading to bed (2:30 here), feel free to edit it if I didn't do it correctly 🙂

@j-f1 j-f1 changed the title fix: do not format functions with arguments as react hooks fix: do not format callbacks with arguments as React hooks Jan 21, 2019
@ikatyang ikatyang merged commit 5657316 into prettier:master Jan 21, 2019
@ikatyang
Copy link
Member

Thanks!

@SimenB SimenB deleted the react-hook-break branch January 21, 2019 16:32
@automatops automatops mentioned this pull request Jan 22, 2019
Merged
1 task
@ghost ghost mentioned this pull request Jan 22, 2019
Merged
1 task
@ikatyang
Copy link
Member

👉 https://github.com/prettier/prettier/releases/tag/1.16.1

@SimenB
Copy link
Contributor Author

SimenB commented Jan 22, 2019

Thank you! Landed the upgrade in Jest as well - no changes from 1.15 🎉

@xcomponentadmin xcomponentadmin mentioned this pull request Jan 22, 2019
Merged
@aleclarson
Copy link

I don't think presence of arguments is the best heuristic here.

Why not detect non-method calls with a name matching /^use[A-Z]\w*$/?

@j-f1
Copy link
Member

j-f1 commented Feb 4, 2019 •
edited
Loading

Maybe it should use the Hook style if foo((...) => { fits on one line, and break the arguments if it doesn’t.

@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label May 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators May 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@suchipi suchipi suchipi approved these changes

@ikatyang ikatyang ikatyang approved these changes

@j-f1 j-f1 j-f1 approved these changes

Assignees
No one assigned
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.
Projects
None yet
Milestone
1.16.1
Development

Successfully merging this pull request may close these issues.
5 participants
@SimenB @ikatyang @aleclarson @j-f1 @suchipi