Hug on single object destructuring function #1022

Merged
merged 1 commit into from Mar 22, 2017

Conversation

Projects
None yet
2 participants
@vjeux
Collaborator

vjeux commented Mar 16, 2017

This is very common for stateful react functional components. It also matches last object expansion. I think that we should only do that for a single argument in function definitions though, unlike the last for function calls.

Fixes #1019

src/printer.js
+ (fun.params[0].type === "ObjectPattern" ||
+ fun.params[0].type === "FunctionTypeParam" &&
+ fun.params[0].typeAnnotation.type === "ObjectTypeAnnotation") &&
+ printed.length === 1) {

This comment has been minimized.

@jlongster

jlongster Mar 17, 2017

Member

Why do this length check if you already do fun.params.length === 1 ?

@jlongster

jlongster Mar 17, 2017

Member

Why do this length check if you already do fun.params.length === 1 ?

This comment has been minimized.

@vjeux

vjeux Mar 17, 2017

Collaborator

There can be fun.rest and probably others as well which are not counted as params (which is super annoying)

@vjeux

vjeux Mar 17, 2017

Collaborator

There can be fun.rest and probably others as well which are not counted as params (which is super annoying)

This comment has been minimized.

@vjeux

vjeux Mar 17, 2017

Collaborator

Okay, only fun.rest. I'll change the condition

@vjeux

vjeux Mar 17, 2017

Collaborator

Okay, only fun.rest. I'll change the condition

This comment has been minimized.

@jlongster

jlongster Mar 17, 2017

Member

We wouldn't want those to pass here though, right? You can't do ...{ prop: value, etc } so we only want to wrap when fun.params.length === 1 I think?

@jlongster

jlongster Mar 17, 2017

Member

We wouldn't want those to pass here though, right? You can't do ...{ prop: value, etc } so we only want to wrap when fun.params.length === 1 I think?

This comment has been minimized.

@vjeux

vjeux Mar 18, 2017

Collaborator
type x = ({a: number}, ...b) => number

has fun.params.length === 1 ({a: number}) and fun.rest (...b). So we want to make sure that it has both checks and cannot rely on a single one.

@vjeux

vjeux Mar 18, 2017

Collaborator
type x = ({a: number}, ...b) => number

has fun.params.length === 1 ({a: number}) and fun.rest (...b). So we want to make sure that it has both checks and cannot rely on a single one.

@jlongster

This comment has been minimized.

Show comment
Hide comment
@jlongster

jlongster Mar 17, 2017

Member

Sounds great!

Member

jlongster commented Mar 17, 2017

Sounds great!

src/printer.js
const isFlowShorthandWithOneArg = (isObjectTypePropertyAFunction(parent) ||
isTypeAnnotationAFunction(parent) || parent.type === "TypeAlias") &&
- fun.params.length === 1 && fun.params[0].name === null && fun.rest === null;
+ fun.params.length === 1 && fun.params[0].name === null && !fun.rest;

This comment has been minimized.

@jlongster

jlongster Mar 20, 2017

Member

Ah nice, thanks, I think that's a lot clearer!

@jlongster

jlongster Mar 20, 2017

Member

Ah nice, thanks, I think that's a lot clearer!

Hug on single object destructuring function
This is very common for stateful react functional components. It also matches last object expansion. I think that we should only do that for a single argument in function definitions though, unlike the last for function calls.

Fixes #1019

@vjeux vjeux merged commit 04eba07 into prettier:master Mar 22, 2017

@Haroenv Haroenv referenced this pull request in algolia/react-instantsearch Apr 16, 2017

Merged

chore(Dependencies): update major ones #41

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