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

Incorrect type definitions for .then #3075

Closed
NicholasBoll opened this issue Jan 7, 2019 · 5 comments
Closed

Incorrect type definitions for .then #3075

NicholasBoll opened this issue Jan 7, 2019 · 5 comments

Comments

@NicholasBoll
Copy link
Contributor

NicholasBoll commented Jan 7, 2019

Current behavior:

According to the documentation for .then, options optionally come first: https://docs.cypress.io/api/commands/then.html

.then(callbackFn)
.then(options, callbackFn)

The type definitions for .then put options optionally after the callback.

Desired behavior:

No type error

Steps to reproduce: (app code and test code)

The following code works at runtime, but has Typescript errors

/// <reference types="Cypress" />
//@ts-check

describe('then', () => {
  it('should allow custom timeout', () => {
    const sleep = time => new Promise(resolve => setTimeout(resolve, time))

    cy.wrap(5000).then({ timeout: 6000 }, sleep) // TS error, but works at runtime
  })
})

Versions

Cypress: 3.1.4

There are 2 ways to fix this:

  1. Fix type definitions to be consistent with documentation and runtime. This is the easiest and has no breaking changes or modifications to runtime code. Nothing will break in the field
  2. Fix runtime to have options after callback. This is consistent with other commands. To prevent breaking changes, both orders would have to be supported using runtime detection of the arguments. The type signature would remain unchanged.
@NicholasBoll
Copy link
Contributor Author

NicholasBoll commented Jan 7, 2019

@bahmutov @brian-mann I already have a solution ready-to-go for option 1. If that is the desired fix, I can create a PR to fix this issue. It is probably not very common to add a custom timeout to .then which is why we didn't noticed before.

@bahmutov
Copy link
Contributor

bahmutov commented Jan 7, 2019

I also vote for option 1 - just keep the same API for types as for current command http://on.cypress.io/then

@jennifer-shehane
Copy link
Member

I vote for option 1 as well.

We previously had someone ask about this here: #1772

@NicholasBoll
Copy link
Contributor Author

PR coming soon

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jan 30, 2019

Released in 3.1.5.

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

No branches or pull requests

3 participants