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

its type should exclude null and undefined #28872

Closed
eostrom-livefront opened this issue Feb 6, 2024 · 2 comments · Fixed by #28904
Closed

its type should exclude null and undefined #28872

eostrom-livefront opened this issue Feb 6, 2024 · 2 comments · Fixed by #28904
Labels
good first issue Good for newcomers type: typings Issue related to Cypress types (for TypeScript)

Comments

@eostrom-livefront
Copy link

eostrom-livefront commented Feb 6, 2024

Current behavior

A Cypress test that relies on its to yield a value that is not null or undefined will be flagged as an error in TypeScript, even though the docs assert that ".its() will automatically retry until it has a property that is not null or undefined."

Desired behavior

Tests shouldn't have to handle values that can't occur.

Test code to reproduce

describe('contrived example', () => {
  specify('log frameElement', () => {
    cy.window()
      .its('frameElement')
      .then(frameElement => {
        cy.log(frameElement.id)
      })
  })
})

This test runs (and passes, for what it's worth). But TypeScript reports:

cypress/e2e/scenarios/contrived.cy.ts:6:16 - error TS18047: 'frameElement' is possibly 'null'.

6         cy.log(frameElement.id)
                 ~~~~~~~~~~~~

In this example, I can work around it by logging frameElement?.id || '', but it would be better not to have to. And although workarounds are probably always possible, they're not always that simple.

(My actual code involves a third-party analytics library that adds a property to window with an object I want to stub, but the property isn't available immediately on page load, so I have to wait for it. In JavaScript, its is great for that! But I'm converting the tests to TypeScript, and TypeScript doesn't know it's great in that way.)

Cypress Version

13.6.4

Node version

18.17.0

Operating System

MacOS 14.2.1

Debug Logs

No response

Other

It looks to me like this can be fixed by making the resulting Chainable subject non-nullable – changing:

    its<K extends keyof Subject>(propertyName: K, options?: Partial<Loggable & Timeoutable>): Chainable<Subject[K]>

to:

    its<K extends keyof Subject>(propertyName: K, options?: Partial<Loggable & Timeoutable>): Chainable<NonNullable<Subject[K]>>

I did that as a local override, and my TypeScript errors went away. That may or may not be the best solution.

@jennifer-shehane jennifer-shehane added type: typings Issue related to Cypress types (for TypeScript) stage: ready for work The issue is reproducible and in scope good first issue Good for newcomers labels Feb 7, 2024
AkshatHotCode added a commit to AkshatHotCode/cypress that referenced this issue Feb 9, 2024
…ress-io#28872)

This fix addresses an issue where the 'its' function could return null or undefined, causing unexpected behavior in certain scenarios. The change introduces a more robust type check to ensure that the returned value is always non-null and non-undefined.
@AkshatHotCode
Copy link
Contributor

@eostrom-livefront Yes, your proposed fix makes sense and is a reasonable solution to address the TypeScript error. By changing the return type of its to Chainable<NonNullable<Subject[K]>>, you are explicitly stating that the property accessed with its will not be null or undefined. This aligns with the behavior documented in Cypress, where it automatically retries until it has a property that is not null or undefined.
Created a PR as well.

jennifer-shehane added a commit that referenced this issue Apr 23, 2024
…#28904)

* refactor: ensure 'its' function type excludes null and undefined (#28872)

This fix addresses an issue where the 'its' function could return null or undefined, causing unexpected behavior in certain scenarios. The change introduces a more robust type check to ensure that the returned value is always non-null and non-undefined.

* resolve linting issues related to TypeScript types

* Update changelog

* resolved lint errors

* Update cli/types/cypress.d.ts

Co-authored-by: Ryan Manuel <ryanm@cypress.io>

* Update cli/types/cypress.d.ts

* move to correct changelog entry

---------

Co-authored-by: Jennifer Shehane <jennifer@cypress.io>
Co-authored-by: Ryan Manuel <ryanm@cypress.io>
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Apr 23, 2024

Released in 13.8.1.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v13.8.1, please open a new issue.

@cypress-bot cypress-bot bot removed the stage: ready for work The issue is reproducible and in scope label Apr 23, 2024
@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Apr 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Good for newcomers type: typings Issue related to Cypress types (for TypeScript)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants