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

bump jquery to 3.1.1 #1229

Merged
merged 23 commits into from
Sep 25, 2019
Merged

bump jquery to 3.1.1 #1229

merged 23 commits into from
Sep 25, 2019

Conversation

jennifer-shehane
Copy link
Member

@jennifer-shehane
Copy link
Member Author

Definitely going to need to reference the upgrade guide for this: https://jquery.com/upgrade-guide/3.0/

@jennifer-shehane
Copy link
Member Author

First issue: jQuery .selector has been removed in 3.0, which is used extensively to identify a node's selector.

Example where .selector is used:

via jQuery

The property .selector was never a reliable indicator of the selector that could be used to obtain the set of elements currently contained in the jQuery set where it was a property, since subsequent traversal methods may have changed the set.

Plugins that need to use a selector string within their plugin can require it as a parameter of the method. For example, a "foo" plugin could be written as $.fn.foo = function( selector, options ) { /* plugin code goes here */ };, and the person using the plugin would write $( "div.bar" ).foo( "div.bar", {dog: "bark"} ); with the "div.bar" selector repeated as the first argument of .foo().

@Saibamen
Copy link
Contributor

Please update your branch to latest develop branch from upstream.

I've tested all examples with JQuery 3.0.0 and only one test didn't pass:
assertions.spec.js: matches unknown text between two elements: Timed out retrying: Expected to find element: 'undefined', but never found it.

@Saibamen
Copy link
Contributor

Saibamen commented Feb 24, 2019

.selectors in cypress\packages\driver:

  • src\cy\assertions.coffee: line 363 (here)
  • src\cy\chai.coffee: lines 181 (here), 232 and 252 (this is JQuery??)
  • src\cy\commands\querying.coffee: line 62 (here)
  • src\cy\commands\traversals.coffee: line 50 (here)
  • test\cypress\integration\commands\traversals_spec.coffee: line 155 (here) and 157
  • test\cypress\integration\commands\assertions_spec.coffee: test "uses $el.selector in expectation" (here)

@bahmutov
Copy link
Contributor

bahmutov commented Sep 4, 2019

some tests are failing with undefined

   1) s1a "before each" hook for "t2a":
-     CypressError: Timed out retrying: Expected to find element: '.does-not-exist', but never found it.
+     CypressError: Timed out retrying: Expected to find element: 'undefined', but never found it.

@cypress
Copy link

cypress bot commented Sep 4, 2019



Test summary

3357 0 47 0


Run details

Project cypress
Status Passed
Commit 81d15f7
Started Sep 25, 2019 3:53 PM
Ended Sep 25, 2019 3:58 PM
Duration 04:42 💡
OS Linux Debian - 9.8
Browser Multiple

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@bahmutov
Copy link
Contributor

bahmutov commented Sep 6, 2019

You can recreate failing tests locally using for example

~/git/cypress/packages/driver on issue-1215-jquery-3
$ node ../../scripts/cypress run --project ./test \
	--spec test/cypress/integration/commands/querying_spec.coffee

@bahmutov
Copy link
Contributor

bahmutov commented Sep 6, 2019

with jQuery 2 if an element is not found we have selector property which we use to form error message

jquery2-not-found-selector

with jQuery 3 change we do not have this property (and we don't have context property)

jquery3-selector-not-found

Tested the returned result using jquery v3 and v2 fiddle

Screen Shot 2019-09-06 at 2 13 07 PM

So we need to either change the jQuery behavior or "remember" the context and selector ourselves and just add them immediately after calling jQuery

Looking at jQuery v3 migration guide https://jquery.com/upgrade-guide/3.0/:

jQuery v3 has removed the selector property from the returned element,
this commit adds it back, making sure the error messages thrown
include the selector string.

See [jQuery v3 upgrade](https://jquery.com/upgrade-guide/3.0/#breaking-change-deprecated-context-and-selector-properties-removed)
@bahmutov
Copy link
Contributor

bahmutov commented Sep 6, 2019

Next missing test: cy.wrap([1, 2]).first() fails while creating a snapshot, because it tries to set attribute data-cypress-el on jQuery element 1

TypeError: Cannot create property 'data-cypress-el' on number '1'
    at Function.prop (jquery.js:7472)
    at attr (jquery.js:7332)
    at access (jquery.js:3908)
    at jQuery.fn.init.attr (jquery.js:7310)
    at Object.createSnapshot (snapshots.coffee:120)

Seems v3 always runs in strict mode https://jquery.com/upgrade-guide/3.0/#breaking-change-jquery-3-0-runs-in-strict-mode, which throws when trying to set attribute on the primitive type

// elem = 1
// name = 'data-cypress-el'
return ( elem[ name ] = value )
'use strict'
1['foo'] = 2
console.log('ended')
// silently runs just fine without "use strict"

But with "use strict" fails

$ node .
/Users/gleb/git/test-cypress-fiddle/index.js:2
1['foo'] = 2
         ^

TypeError: Cannot create property 'foo' on number '1'

@bahmutov
Copy link
Contributor

bahmutov commented Sep 6, 2019

Snapshots CSS test failing because $.load was removed https://jquery.com/upgrade-guide/3.0/#breaking-change-load-unload-and-error-removed

1) driver/src/cy/snapshots_css .getStyleIds "before each" hook for "returns IDs for cached CSS contents":
     TypeError: url.indexOf is not a function

Because this error occurred during a 'before each' hook we are skipping the remaining tests in the current suite: 'driver/src/cy/snapshots_css'

Changing to the recommended solution on('load', fn)

@bahmutov
Copy link
Contributor

bahmutov commented Sep 6, 2019

Since snapshot test remains - the screenshot height is different from the snapshot value. See https://circleci.com/gh/cypress-io/cypress/150034

chrisbreiding
chrisbreiding previously approved these changes Sep 16, 2019

iframesSizeNode.style.width = `${Math.min($window.width(), $iframesSizeNode.width())}px`
iframesSizeNode.style.height = `${Math.min($window.height(), $iframesSizeNode.height())}px`
iframesSizeNode.style.width = `${Math.min(window.innerWidth, iframesSizeNode.offsetWidth)}px`
Copy link
Contributor

Choose a reason for hiding this comment

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

jQuery 3 changes how it does width and height calculations (it now uses getBoundingClientRect), which takes into account the scaling. We want the absolute value without scaling, so we're better off directly using the DOM properties.

@jennifer-shehane jennifer-shehane changed the title WIP: bump jquery to 3.0 bump jquery to 3.0 Sep 16, 2019
@jennifer-shehane
Copy link
Member Author

@chrisbreiding I'm in favor of bumping it back down to 3.0.0 and pushing that out.

jQuery 3 changed how it does width and height calculations (it now uses getBoundingClientRect), which takes into account the scaling and returns non-integers. We want the absolute integer value without scaling, so we're better off directly using the DOM properties.
@chrisbreiding
Copy link
Contributor

Was able to get 3.1.1 passing.

@jennifer-shehane
Copy link
Member Author

Good team effort. Almost 2 years coming.

@chrisbreiding I guess open a new PR for jQuery latest. Merging this one in.

@jennifer-shehane jennifer-shehane merged commit 2c35402 into develop Sep 25, 2019
@jennifer-shehane jennifer-shehane changed the title bump jquery to 3.0 bump jquery to 3.1.1 Sep 25, 2019
@wKovacs64
Copy link

Yes! 🙌 Is a release imminent?

grabartley pushed a commit to grabartley/cypress that referenced this pull request Oct 6, 2019
* bump jquery to 3.0

* add selector property to the element returned by jQuery query

jQuery v3 has removed the selector property from the returned element,
this commit adds it back, making sure the error messages thrown
include the selector string.

See [jQuery v3 upgrade](https://jquery.com/upgrade-guide/3.0/#breaking-change-deprecated-context-and-selector-properties-removed)

* do not try setting highlight attribute on non-elements

* use jquery v3 on load callback

* another instance of jquery load

* better element check before setting an attribute

* use dom APIs instead of jQuery when unscaling AUT for screenshot

* bump jquery to 3.4.1

* replace instances of $.fn.width/height with respective dom APIs

jQuery 3 changed how it does width and height calculations (it now uses getBoundingClientRect), which takes into account the scaling and returns non-integers. We want the absolute integer value without scaling, so we're better off directly using the DOM properties.

* try should instead of then

* Revert "try should instead of then"

This reverts commit 91fdbde.

* try getting rid of borders

* try jquery 3.3.1

* try jquery 3.2.1

* try jquery 3.1.1
@chrisbreiding chrisbreiding deleted the issue-1215-jquery-3 branch April 5, 2022 18:28
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.

Upgade jQuery dependency to 3.x
5 participants