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

Chained calls with object argument indentation #257

Closed
albertorestifo opened this issue Sep 13, 2015 · 9 comments

Comments

@albertorestifo
Copy link

commented Sep 13, 2015

Before the latest Standard update I could do this:

promiseFooBar({
    foo: true,
    bar: false
  })
  .then(fooBar => console.log(fooBar))
  .catch(err => console.log(err))

Now the new indentation rules forces me to do either of the following:

// 1
promiseFooBar({
  foo: true,
  bar: false
})
  .then(fooBar => console.log(fooBar))
  .catch(err => console.log(err))

// 2
promiseFooBar({
  foo: true,
  bar: false })
  .then(fooBar => console.log(fooBar))
  .catch(err => console.log(err))

// 3
promiseFooBar({
  foo: true,
  bar: false
}).then(fooBar => console.log(fooBar))
  .catch(err => console.log(err))

I was very happy using the first example, now not allowed. I understand and respect that the Standard rules are not up to discussion, so I just want to ask:

What way should I indent them?

@albertorestifo albertorestifo changed the title Chained calls object indentation Chained calls with object argument indentation Sep 13, 2015

@albertorestifo

This comment has been minimized.

Copy link
Author

commented Sep 13, 2015

Forgot one, that's what I've been using since the update

// 4
const options = { foo: true, bar: false }

promiseFooBar(options)
  .then(fooBar => console.log(fooBar))
  .catch(err => console.log(err))
@yoshuawuyts

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2015

they all lgtm except:

promiseFooBar({
  foo: true,
  bar: false })
  .then(fooBar => console.log(fooBar))
  .catch(err => console.log(err))
@pbrinkmeier

This comment has been minimized.

Copy link

commented Sep 14, 2015

What about that (is more semantic IMO):

promiseFooBar({
  foo: true,
  bar: false
})
.then(fooBar => console.log(fooBar))
.catch(err => console.log(err))
@albertorestifo

This comment has been minimized.

Copy link
Author

commented Sep 14, 2015

@reg4in It makes it harder to detect a chained call when looking at a lot of code.

But yeah, it also satisfies the Standard rules, so it's a valid option

@pbrinkmeier

This comment has been minimized.

Copy link

commented Sep 14, 2015

@albertorestifo I personally don't think so but I won't force anyone to use it 😛

@dcousens

This comment has been minimized.

Copy link
Member

commented Sep 14, 2015

Before the latest Standard update I could do this:

Which versions specifically?

@emgeee

This comment has been minimized.

Copy link

commented Sep 15, 2015

I did some digging into this and it looks to have cropped up starting in v5.0.0 when the indent rule got stricter. As far as I can tell, this is an issue relating to how eslint handles the indent rule and isn't something standard can do much about.

There looks to be an open issue for eslint here: eslint/eslint#3614 that discusses adding more customization around indentation and chaining.

@albertorestifo

This comment has been minimized.

Copy link
Author

commented Sep 15, 2015

@emgeee 👍 Exactly, update to 5.0.0 is what did it for me

@feross

This comment has been minimized.

Copy link
Member

commented Sep 22, 2015

Yeah, unfortunately eslint reimplemented the indent rule and it got stricter in some edge cases.

There's nothing we can do about it.

@feross feross closed this Sep 22, 2015

@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
6 participants
You can’t perform that action at this time.