-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Document CT changes for v11 #4828
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is still a work in progress, but added a few comments for now.
Also, should there be an example for Vue on how to test "unmount" functionality for a component like there is for React?
version 11.0. | ||
[See the full changelog for version 10.0](/guides/references/changelog#11-0-0). | ||
|
||
### Component Testing Changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the second Component Testing Changes
section within this page, which causes the latter to be incremented to make it unique e.g. https://cypress-documentation-gdkinysy2-cypress-io.vercel.app/guides/references/migration-guide#Component-Testing-Changes-1
Doesn't look like we have an on-link attached to this and seems like an edge-case, just wanted to point it out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed this as well, I changed it to "Component Testing Updates" to make it unique and pushed a corresponding commit to your services PR.
Co-authored-by: Stokes Player <[email protected]>
…ss-io/cypress-documentation into release-11-migration-guide
We never had |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall lgtm, just left a couple of questions
### Component Testing Updates | ||
|
||
As of Cypress 11, Component Testing is now generally available. Learn more about | ||
what generally available implies [here](link_to_post) and the improvements |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whats the generally available post?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought were we going to post a definition of GA but looks like we aren't doing that anymore (at least before GA).
Removed.
#### After - Cypress 11 and `unmountComponentAtNode` | ||
|
||
```js | ||
import { getContainerEl } from 'cypress/react' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is getContainerEl new? It doesn't appear to be in v10. Why wouldn't they just use the component
that the mount returns?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It actually is in v10, it's from cypress/mount-utils
. https://github.com/cypress-io/cypress/blob/develop/npm/mount-utils/src/index.ts#L42. It isn't documented but it how we write and ensure all our mount adapters target the same element.
You can't unmount a component, you need to pass the exact same HTML element you mounted on (this is how React works). It's the argument called container here: https://reactjs.org/docs/react-dom.html#unmountcomponentatnode.
I personally think just keeping umount
would have been far more ergonomic, but this shouldn't be too disruptive - unmount
isn't that commonly used, from what I can see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should add some tsdocs around getContainerEl
since its a part of our api now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updates look good. Just missing those external links when they are available.
Links are all updated. I think this is ready to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some minor comments
Updated, thanks @astone123 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added two missing commas. Everything looks good to go to me.
Short version of the CT changes in Cypress 11. A more detailed article will be posted that explains out rationale and provides some additional insights: https://docs.google.com/document/d/1n4H0ADl0QNBWqbtGiKg4AzC0v8pQ8wZL0YLZeOCOg94/edit# (Cypress team only for now, until published).