-
Notifications
You must be signed in to change notification settings - Fork 20.5k
Deferred: Remove default callback context #3061
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
Deferred: Remove default callback context #3061
Conversation
This reverts commit 9776354.
|
Deeper and deeper... af85a48 broke the Promises/A+ strict-mode assertions, and there are two ways to fix it. The simpler (and smaller) of the two is just letting jQuery run in strict mode, and the more complex involves preserving My personal inclination is to go strict mode, with justification documented in
However, three years ago trac-13335 was considered a big enough deal that we removed "use strict" just three weeks after introducing it, and today every browser engine except Blink throws an exception when a non-strict function tries to access a strict caller. The question is, has enough time elapsed that we now feel comfortable with reintroducing it and wontfixing any reports about ill-advised code that tries to naïvely |
|
Id really love us to go to strict mode if possible. The concerns about tracing strict code from sloppy one are permanent as AFAIK the spec forbids such access. We're thinking about moving to ES6 modules one day; if we're going to expose them similarly to how we expose our AMD modules in I'd also like to point out a comment from one Chromium issue (https://bugs.chromium.org/p/v8/issues/detail?id=222#c13):
More & more libraries use strict mode. Angular, Ember & React do so if you use a JS framework you most likely already have it enabled somewhere. Bootstrap & Foundation do so if you use a popular CSS framework it's taken care of as well. Three.js enables strict mode as well. I'd argue the majority of our users might be already using strict mode via various libraries that they use. Us not doing that won't help it but it allows external code to mess with jQuery more, introduces potential security concerns and puts us further from ES6 modules. |
Although we still support IE 9 which doesn't support strict mode (AFAIK the only browser we still support that doesn't) and our test suite should still pass in them so perhaps there's no way around that just via enabling strict mode... |
|
We can try it again and release 3.0.1 if users wouldn't like it
As i understand those tests expect for context to be |
…but doesn't require throwing an exception. Chrome, for example, just returns a
Right, which serves as good defense for jQuery doing so as well.
But that's fine; it just means
It's not a bug. At any rate, it looks like jumping into strict mode is the way to go for 3.0. I'll land it later today. |
By that i meant that test should check what mode they are in, if strict - then assertion should check for Otherwise, those tests suggesting that Promise/A+ spec is not compatible with sloppy mode, which is not true. So
That exactly what they should be testing |
|
Like it said in https://promisesaplus.com/#point-69
|
|
In any case, i'm not oppose to land, but if we would have issues with strict mode again, then we would have to revert so we would face that issue all over again. |
They check both, explicitly: https://github.com/promises-aplus/promises-tests/blob/0d409df278345753c9527539a5558db67d42bb02/lib/tests/2.2.5.js |
|
I'm good with another shot at |
|
Remember that we now need not only to uncomment the pragma in wrapper.js Michał Gołębiowski |
This commit increases the gzipped size by 90 bytes so a better solution is needed but, at the same time, it disables the very optimizations that are causing strict mode violations in Firefox 45, Safari 9 & IE 10. Refs 7608437 Refs mishoo/UglifyJS#1052
|
I added a note in the upgrade guide. |
Fixes gh-3060