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

.within() now throws an error if given more than one subject #4898

Merged
merged 3 commits into from
Dec 5, 2022

Conversation

BlueWinds
Copy link
Contributor

Related code PR: cypress-io/cypress#24975

This brings the docs in line with the new behavior.

@vercel
Copy link

vercel bot commented Dec 5, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
cypress-documentation ✅ Ready (Inspect) Visit Preview Dec 5, 2022 at 9:51PM (UTC)

@marktnoonan
Copy link
Contributor

Already approved but actually how about updating the examples a little bit:

Screen Shot 2022-12-05 at 3 32 05 PM

cy.get('.className') is only safe when it happens to yield just one element. Maybe we should use something like contains() that always yields one thing, or add a .first(). And under "Incorrect Usage" we could add an example where multiple things are yielded by the previous command.

@@ -11,6 +11,11 @@ _Released MM/DD/YYYY_
- Cypress now throws an error if commands are invoked from inside a `.should()`
callback. This previously resulted in unusual and undefined behavior; it is
now explicitly an error.
- The `.within()` command now throws an error if given more than one DOM element
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BlueWinds Can you add a migration guide entry on how to resolve this error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added migration guide.

@BlueWinds
Copy link
Contributor Author

cy.get('.className') is only safe when it happens to yield just one element. Maybe we should use something like contains() that always yields one thing, or add a .first(). And under "Incorrect Usage" we could add an example where multiple things are yielded by the previous command.

Updated the usage section.

Correct Usage

cy.get('.list').first().within(($list) => {}) // Yield the first `.list` and scope all commands within it

Incorrect Usage

cy.within(() => {}) // Errors, cannot be chained off 'cy'
cy.getCookies().within(() => {}) // Errors, 'getCookies' does not yield DOM element
cy.get('div').within(($divs) => {}) // Probably errors, because get('div') yields multiple elements

@@ -310,6 +310,69 @@ cy.get('button')
.then(res => { ...handle response... })
```

#### `.within()`

[`.within()`](/api/commands/within) now throws an error if it is passed multiple
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[`.within()`](/api/commands/within) now throws an error if it is passed multiple
The [`.within()`](/api/commands/within) command now throws an error if it is passed multiple

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied this change to all the bits in this section (.invoke() and .should() were phrased the same way).

[`.within()`](/api/commands/within) now throws an error if it is passed multiple
elements as the subject. This previously resulted in inconsistent behavior,
where some commands would use all passed in elements, some would use only the
first and ignore the rest, and `.screenshot()` would throw an error.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add link to screenshot cmd

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

- })
+ .as('rows') // Store multiple elements as an alias

+cy.get('@rows').contains('foo')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wouldn't they limit the contains selector?

Suggested change
+cy.get('@rows').contains('foo')
+ cy.contains('tr', 'foo')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change the example; this works for .contains, but they might be running assertions, performing actions, or doing anything else that can't be used in this way, which was my intent in this example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(any set of commands using .within() can be written without .within(). It doesn't provide any unique functionality)

Comment on lines 363 to 375
invoked from inside a `.should()` callback. This previously resulted in unusual
and undefined behavior. If you wish to execute a series of commands on the
yielded value, use`.then()` instead.

```diff
cy.get('button')
- .should(($button) => {

})
+ .then(api => api.makeARequest('http://example.com'))
.then(res => { ...handle response... })
```

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy paste?

@BlueWinds BlueWinds merged commit 0dcf3a0 into v12 Dec 5, 2022
@BlueWinds BlueWinds deleted the pr-24975-one-within-subject branch December 5, 2022 22:21
debrisapron pushed a commit that referenced this pull request Dec 6, 2022
* .within() now throws error when passed more than one subject.

* Add migration guide, update based on reviews
mjhenkes added a commit that referenced this pull request Dec 6, 2022
* Remove pages and references to functionality obsoleted by multidomain GA

* fix: Explain error thrown when cypress commands in .should() callback (#4755)

* fix: Explain error thrown when cypress commands in .should() callback

* Improve layout of previous changes and provide second example of how to fix

* Update content/api/commands/should.md

Co-authored-by: Rachel <rachel@cypress.io>

* Apply suggestions from code review

Co-authored-by: Zach Bloomquist <git@chary.us>

* Run prettier

* Run prettier again...?

* One more prettier run... :/

Co-authored-by: Rachel <rachel@cypress.io>
Co-authored-by: Zach Bloomquist <git@chary.us>

* docs: removing Cookies.defaults/preserveOnce (#4779)

* docs: remove experimentalSessionAndOrigin (#4807)

* Update cookie commands domain option description (#4861)

* docs: Queries, Detached DOM, and Retry-Ability (#4835)

* First rework of retryability guide

* Update each command's Yields section, and all guides, with information about queries vs commands

* Add Custom Queries page

* Minor formatting tweaks

* Review changes

* Review updates

* Update based on review + last week meetings

* More review updates

* Fix tests

* breaking: drop node 12, 13, 15 and 17 support (#4879)

* Add docs for new local/session storage commands (#4876)

* feat: update okta login guide for realworld app (#4883)

* feat: update okta login guide for realworld app

* chore: make changes to okta to have parity with cognito changes

* chore: address code review comments

* feat: update cognito login guide for realworld app (#4882)

* feat: update cognito login guide for realworld app

* chore: update guide from comments in code review

* properly close alert tag

* Update content/guides/end-to-end-testing/amazon-cognito-authentication.md

* chore: address comments from code review

* fix linting

* v12 Migration Guide (#4862)

Co-authored-by: Matt Schile <mschile@cypress.io>
Co-authored-by: Blue F <blue@everblue.info>
Co-authored-by: DEBRIS APRON <debrisapron@gmail.com>
Co-authored-by: Ben M <benm@cypress.io>
Closes undefined

* Small update to cy.origin API docs for v12

* Update auth examples for v12 on custom commands page

* 12: update test isolation docs to use true/false instead of on/off (#4890)

Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>
Co-authored-by: Matt Henkes <mjhenkes@gmail.com>
Co-authored-by: Bill Glesias <bglesias@gmail.com>

* docs: add documentation for experimentalOriginDependencies (#4897)

* Documentation updates for v12 (#4880)

* re-add websecurity, links to websecurity, and trade-offs guides

* chore: revamp documentation around web security page

* chore: update same-origin tradeoff to be new navigation rules, including our SD chart, to help paint users a clear picture with cy.origin

* chore: link to the experimental modify obstructive third party code doc in web security from origin

* chore: update Error Messages section to reflect allowing cross origin visiting

* update best practices: visiting external sites

* remove node 12 from installing cypress section

* chore: update key differences to plug session and origin over programmatic login

* chore: update with suggestions from code review

* add okta/amazon guide links in trade-offs and update workarounds

* feat: add cross origin testing guide

* update image for command time out with visit

* chore: readd legacy errors and add a Note section to explain that this is only for cypress v11 and under

* chore: update suggestions from code review

* chore: add suggestions from code review

* fix: fix okta alert banner (needed a new line)

* fix: broken image in error messages

* chore: update error header for on link to address cypress-io/cypress-services#5040 (comment)

* Update cy.session API docs for v12 (#4851)

Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>
Closes #4507

* Remove pages and references to functionality obsoleted by multidomain GA

* fix: Explain error thrown when cypress commands in .should() callback (#4755)

* fix: Explain error thrown when cypress commands in .should() callback

* Improve layout of previous changes and provide second example of how to fix

* Update content/api/commands/should.md

Co-authored-by: Rachel <rachel@cypress.io>

* Apply suggestions from code review

Co-authored-by: Zach Bloomquist <git@chary.us>

* Run prettier

* Run prettier again...?

* One more prettier run... :/

Co-authored-by: Rachel <rachel@cypress.io>
Co-authored-by: Zach Bloomquist <git@chary.us>

* docs: removing Cookies.defaults/preserveOnce (#4779)

* docs: remove experimentalSessionAndOrigin (#4807)

* Update cookie commands domain option description (#4861)

* docs: Queries, Detached DOM, and Retry-Ability (#4835)

* First rework of retryability guide

* Update each command's Yields section, and all guides, with information about queries vs commands

* Add Custom Queries page

* Minor formatting tweaks

* Review changes

* Review updates

* Update based on review + last week meetings

* More review updates

* Fix tests

* breaking: drop node 12, 13, 15 and 17 support (#4879)

* Add docs for new local/session storage commands (#4876)

* feat: update okta login guide for realworld app (#4883)

* feat: update okta login guide for realworld app

* chore: make changes to okta to have parity with cognito changes

* chore: address code review comments

* feat: update cognito login guide for realworld app (#4882)

* feat: update cognito login guide for realworld app

* chore: update guide from comments in code review

* properly close alert tag

* Update content/guides/end-to-end-testing/amazon-cognito-authentication.md

* chore: address comments from code review

* fix linting

* v12 Migration Guide (#4862)

Co-authored-by: Matt Schile <mschile@cypress.io>
Co-authored-by: Blue F <blue@everblue.info>
Co-authored-by: DEBRIS APRON <debrisapron@gmail.com>
Co-authored-by: Ben M <benm@cypress.io>
Closes undefined

* 12: update test isolation docs to use true/false instead of on/off (#4890)

Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>
Co-authored-by: Matt Henkes <mjhenkes@gmail.com>
Co-authored-by: Bill Glesias <bglesias@gmail.com>

* docs: add documentation for experimentalOriginDependencies (#4897)

* Documentation updates for v12 (#4880)

* re-add websecurity, links to websecurity, and trade-offs guides

* chore: revamp documentation around web security page

* chore: update same-origin tradeoff to be new navigation rules, including our SD chart, to help paint users a clear picture with cy.origin

* chore: link to the experimental modify obstructive third party code doc in web security from origin

* chore: update Error Messages section to reflect allowing cross origin visiting

* update best practices: visiting external sites

* remove node 12 from installing cypress section

* chore: update key differences to plug session and origin over programmatic login

* chore: update with suggestions from code review

* add okta/amazon guide links in trade-offs and update workarounds

* feat: add cross origin testing guide

* update image for command time out with visit

* chore: readd legacy errors and add a Note section to explain that this is only for cypress v11 and under

* chore: update suggestions from code review

* chore: add suggestions from code review

* fix: fix okta alert banner (needed a new line)

* fix: broken image in error messages

* chore: update error header for on link to address cypress-io/cypress-services#5040 (comment)

* Update auth examples for v12 on custom commands page

* Small update to cy.origin API docs for v12

* Update cy.session API docs for v12 (#4851)

Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>
Closes #4507

* chore: address docs feedback post merge (#4899)

* .within() now throws an error if given more than one subject (#4898)

* .within() now throws error when passed more than one subject.

* Add migration guide, update based on reviews

* Update Logging In section of Testing Your App page (#4885)

Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>
Closes #4498

* Update End-to-End Testing -> Auth0 Authentication docs for v12 (#4895)

Co-authored-by: Bill Glesias <bglesias@gmail.com>

* Cypress.Session Cypress API (#4900)

* docs around Cypress.session api

* data not date

* Update content/api/cypress-api/session.md

Co-authored-by: Matt Henkes <mjhenkes@gmail.com>

* Update content/api/cypress-api/session.md

Co-authored-by: Matt Henkes <mjhenkes@gmail.com>

* Update content/api/cypress-api/session.md

Co-authored-by: Matt Henkes <mjhenkes@gmail.com>

* Update content/api/cypress-api/session.md

Co-authored-by: Matt Henkes <mjhenkes@gmail.com>

* fix markdown

* Update content/api/cypress-api/session.md

* Apply suggestions from code review

Co-authored-by: Matt Henkes <mjhenkes@gmail.com>

* V12 ChangeLog (#4896)

Co-authored-by: Matt Schile <mschile@cypress.io>
Co-authored-by: Blue F <blue@everblue.info>
Co-authored-by: DEBRIS APRON <debrisapron@gmail.com>
Co-authored-by: Ben M <benm@cypress.io>
Co-authored-by: Ryan Manuel <ryanm@cypress.io>
Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>

Co-authored-by: DEBRIS APRON <debrisapron@gmail.com>
Co-authored-by: Blue F <blue@everblue.info>
Co-authored-by: Rachel <rachel@cypress.io>
Co-authored-by: Zach Bloomquist <git@chary.us>
Co-authored-by: Matt Schile <mschile@cypress.io>
Co-authored-by: Matt Henkes <mjhenkes@gmail.com>
Co-authored-by: Chris Breiding <chrisbreiding@users.noreply.github.com>
Co-authored-by: Bill Glesias <bglesias@gmail.com>
Co-authored-by: Ben M <benm@cypress.io>
Co-authored-by: Ryan Manuel <ryanm@cypress.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants