Skip to content

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

Merged
ikatyang merged 5 commits intoprettier:masterfrom
SimenB:react-hook-break
Jan 21, 2019
Merged

fix: do not format callbacks with arguments as React hooks#5778
ikatyang merged 5 commits intoprettier:masterfrom
SimenB:react-hook-break

Conversation

@SimenB
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

ikatyang commented Jan 21, 2019

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
Copy Markdown
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
Copy Markdown
Member

ikatyang commented Jan 21, 2019

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
Copy Markdown
Member

ikatyang commented Jan 22, 2019

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

@SimenB
Copy link
Copy Markdown
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
Copy Markdown

aleclarson commented Feb 4, 2019

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
Copy Markdown
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