-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add support for Cypress 13 #143
Conversation
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.
Apparently Cypress 13 is no longer working with node 14. The matrix build under https://github.com/4teamwork/cypress-drag-drop/blob/master/.github/workflows/test.yml#L11 is still referencing node 14, that is why the tests are failing. Can you please remove node version 14 and probably add version 18 instead in the matrix build. Thank you so much for your contribution.
package.json
Outdated
@@ -48,6 +48,6 @@ | |||
"webpack-dev-server": "^3.11.0" | |||
}, | |||
"peerDependencies": { | |||
"cypress": "^2.1.0 || ^3.1.0 || ^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0 || ^12.0.0" | |||
"cypress": "^2.1 || ^3.1 || 4 - 13" |
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 like the version range annotation. Is it also possible to do 2 - 13
? Or is it necessary that 2.1
and 3.1
are listed separately?
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 can go straight to 2 - 13
if you like. I just tried to keep things the same in terms of what versions would match.
The current range excludes any of the 2.0.x
and 3.0.x
versions, instead it will only match 2.1.x
or 3.1.x
and up within the same major version. You can see which versions are matched with NPM's SemVer Calculator.
If you don't mind signaling support for these older versions I can make the change, just let me know.
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 think the explicit support for 2.1.x
and 3.1.x
was introduced by accident a long time ago. IMO having 2 - 13
is much better. Thank you for pointing that 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.
Upon closer consideration, perhaps it is time to start dropping support for these older versions? They are not actually being tested against on CI, which makes it highly doubtful that this library is actually still compatible with these older versions.
Then again I could be wrong about that, I have no idea how much (or little) the API for Cypress has changed. Perhaps this is also better served as a separate issue/discussion.
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.
Great, updated the version range as discussed 👍
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.
You are absolutely right. We are always trying to move forward and support the latest version of Cypress. At least, the tests running on CI, make sure that it works with the latest version of Cypress currently supported in the plugin. As long as we don't have to make changes in the plugin where we talk to the Cypress API, the backwards compatibility should still be there. But ensuring that, would mean running the tests against different Cypress versions, which is not feasible.
As far as I know, Cypress does not have a list of LTS versions, so it is hard to tell which versions we can drop.
Done 👍 |
Hmmm, looks like the version of WebPack that is being used there is not compatible with Node.js version 18. Perhaps it would be best to handle this in a separate PR, I'll remove version 18 from the matrix for now. |
Hi, When new version of this package will release with cypress 13 support? |
A release would be really appreciated! |
Version |
thanks! |
Adds Cypress 13 as a supported version, closes #142.