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

$.holdReady is broken in 3.2.0 #3573

Closed
San4es opened this issue Mar 17, 2017 · 8 comments
Closed

$.holdReady is broken in 3.2.0 #3573

San4es opened this issue Mar 17, 2017 · 8 comments

Comments

@San4es
Copy link

@San4es San4es commented Mar 17, 2017

The "$.holdReady is not a function" error occurs after the update to jQuery 3.2.0.

To reproduce the issue, see https://jsfiddle.net/4ocpkd69/ (see browser console) and compare results with jQuery 3.1.1 in https://jsfiddle.net/4ocpkd69/1/

@mgol mgol added this to the 3.2.1 milestone Mar 17, 2017
@mgol
Copy link
Member

@mgol mgol commented Mar 17, 2017

It was moved erroneously to jQuery.fn.holdReady in bd984f0. Marking as a blocker for 3.2.1.

Thanks for the report!

@mgol
Copy link
Member

@mgol mgol commented Mar 17, 2017

This only shows we should have explicit tests for any API that we deprecate, even if there weren't any due to legacy reasons.

@timmywil
Copy link
Member

@timmywil timmywil commented Mar 17, 2017

Oof. How did we not catch this?

By the way, temporary workaround:

jQuery.holdReady = jQuery.fn.holdReady;
@timmywil
Copy link
Member

@timmywil timmywil commented Mar 17, 2017

I think making the necessary adjustments to this PR and landing it will give us the test coverage we want for this issue.

@Krinkle
Copy link
Member

@Krinkle Krinkle commented Mar 17, 2017

It had been failing for several weeks in Jenkins/TestSwarm:
http://swarm.jquery.org/project/jquerymigrate

screen shot 2017-03-17 at 12 06 41

screen shot 2017-03-17 at 12 05 30

Core commit (17 February):

  • bd984f0 Core: Move holdReady to deprecated
@timmywil
Copy link
Member

@timmywil timmywil commented Mar 17, 2017

Well, that's jQuery migrate, not jQuery core, but I'm surprised no one said anything.

@mgol
Copy link
Member

@mgol mgol commented Mar 18, 2017

@timmywil
Copy link
Member

@timmywil timmywil commented Mar 18, 2017

This seems like a rare case, and it will be fixed when we land the PR I mentioned. I would like to ensure that reviewing core tests is sufficient for releasing core and reviewing migrate tests sufficient for migrate.

timmywil added a commit to timmywil/jquery that referenced this issue Mar 18, 2017
timmywil added a commit to timmywil/jquery that referenced this issue Mar 18, 2017
timmywil added a commit to timmywil/jquery that referenced this issue Mar 18, 2017
timmywil added a commit to timmywil/jquery that referenced this issue Mar 18, 2017
@timmywil timmywil closed this in fcc9a9e Mar 20, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.