Problem with no-callback-literal

[saratitan]

I fail to see what's invalid with the following code, what makes this callback literal?

  ], (err, result) => {
    callback(...response(err, (result ? result.paymentProvider : '')))
  })

results in

Unexpected literal in error position of callback  standard/no-callback-literal

[rkurbatov]

Confirm, it complains about callback with rested arguments.

[toddself]

Also, it is complaining about:

    if (err) {
      log.error(err, 'Cannot get from database')
      return cb({
        message: 'Cannot get data',
        code: 500
      })
    }

We use this frequently since we don't actually want the end user to see the backend message since it is 100% useless to them (and could potentially leak table names, etc), so we return an object that our HTTP framework understands.

[LinusU]

@toddself We have a similar setup at work, but we still use Error instances instead of a plain object. Something like this:

class UserError extends Error {
  constructor (...args) {
    super(...args)
    this.name = this.constructor.name
    Error.captureStackTrace(this, this.constructor)
  }
}

app.use((err, req, res, next) => {
  if (err instanceof UserError) {
    // send nice detailed error
  } else {
    // send generic 500 and log error
  }
})

[toddself]

This was easy to fix by just moving the object creation to a new variable and returning that variable.

Not sure if this is a bad thing though?

[maxbrunsfeld]

I think this rule should simply be removed. Not all callbacks need to take errors as their first argument.

[bcomnes]

I’m guessing the rule semantics didn’t take spread operators into account. Perhaps there is a newer version of he rule?

[catdaddy23]

I agree. It should be removed all together

…

Sent from my iPhone

On Jan 5, 2018, at 5:12 PM, Bret Comnes ***@***.***> wrote: I’m guessing the rule semantics didn’t take spread operators into account. Perhaps there is a newer version of he rule? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[LinusU]

I agree. It should be removed all together

I really don't think so. E.g. the code posted by @toddself is exactly the code we want to specifically want to disallow, since it breaks assumptions that people have on how Node.js-style callbacks works.

I’m guessing the rule semantics didn’t take spread operators into account.

This problem would be nice to fix though, this should be reported upstream to ESLint, anyone feel up for doing that?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[feross]

@LinusU This problem would be nice to fix though, this should be reported upstream to ESLint, anyone feel up for doing that?

The problem is that we're the maintainers of the no-callback-literal rule ;) https://github.com/standard/eslint-plugin-standard

[feross]

Let's move discussion here: #1352