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

Don't allow assigning return values to objects #20

Open
jeremy-ebler-vineti opened this issue May 17, 2019 · 4 comments
Open

Don't allow assigning return values to objects #20

jeremy-ebler-vineti opened this issue May 17, 2019 · 4 comments
Labels

Comments

@jeremy-ebler-vineti
Copy link

I have a team that's started writing Cypress like:

context('Foo', () => {
  const scope = {}
  it('allows this', () => {
    scope.baddie = cy.get('it')
  })
  ...
})

I was expecting no-assigning-return-values to error out on the assignment of a cypress command to scope.baddie.

const scope = {} is also discouraged by Cypress's Docs, but that's another issue.

@brettz9
Copy link
Contributor

brettz9 commented Feb 17, 2020

For whoever might work on implementing this change, there are plenty of other contexts where a return value gets used, e.g., within a template, as part of a binary expression, etc. etc.

In our work on eslint-plugin-unicorn, we found that rather than trying to blacklist the various other contexts where the return value might be used besides variable declarations (discussed more fully at sindresorhus/eslint-plugin-unicorn#503 ), that a simple utility could be used to whitelist only those cases where the return values are known not to be used, i.e.:

module.exports = ({parent}) => !parent || parent.type === 'ExpressionStatement';

Then just call this method with the node in question, e.g., isValueNotUsable(node) to see whether this is safe (and an error can be reported otherwise).

You may need to expand though to check the ancestor chain to allow for chained commands like cy.abc().def() since you don't want those to be allowed.

@brettz9
Copy link
Contributor

brettz9 commented Feb 17, 2020

By the way, this proposed expanding to other contexts could help catch another problem, namely that Cypress doesn't seem to like await to be used (though I wonder whether this can be liberalized to be permitted where a Promise is explicitly allowed, i.e., with cy.task).

@brettz9
Copy link
Contributor

brettz9 commented Feb 27, 2020

Or if cypress-io/cypress#6575 is implemented, to liberalize returning a (Promise) value from a known custom command.

jaffrepaul pushed a commit that referenced this issue Sep 27, 2022
…lobals-13.16.0

chore(deps): bump globals from 13.15.0 to 13.16.0
@MikeMcC399 MikeMcC399 added the bug label Apr 17, 2024
@MikeMcC399
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants