This repository was archived by the owner on Sep 6, 2021. It is now read-only.
Repository navigation
This repository was archived by the owner on Sep 6, 2021. It is now read-only.
Upgrade jQuery #3072
Description
I'm not sure if it's been discussed at all, but Brackets is still on jQuery 1.7 and there would be some speed and stability benefits of upgrading to jQuery 1.9.1.
Unfortunately, some deprecated functions are still being used in core so the jQuery migrate plugin would need to be included as well.
I'm not sure what the intended browser support is for Brackets, but jQuery 2.0 may also be a good option (decreased size, deprecated functions removed, customizable), but drops support for IE 6, 7 & 8.
I attempted to run the unit tests after swapping out jQuery-1.7 with jQuery-1.9.1 and including the jQuery Migrate plugin, but unit tests are failing for other reasons right now (#3070).
Activity
peterflynn commentedon Mar 8, 2013
@TuckerWhitehouse can you list out which deprecated functions we're using? That would be a good start to cleaning up the code -- ideally we'd do a clean migration instead of relying on a shim.
TuckerWhitehouse commentedon Mar 8, 2013
Certainly, it'll be a little bit easier once the unit tests bug is solved.
I know one thing was the Node Connection is using the deferred.isRejected() which was deprecated in 1.7 and removed in 1.8 (http://api.jquery.com/deferred.isRejected/), should be using deferred.state()==="rejected"
Edit:
A quick "Find in Files" for ".isRejected" shows that it is actually used in
I can go through and start replacing these and submit a pull request
pthiess commentedon Mar 11, 2013
Reviewed, we thought it was ok to make changes and add a pull request to support 1.9.1 but only if it wouldn't break anything in 1.7. Changing our implementation basis would require significant testing, therefore we move it to backlog. Added card 813. Closing in here eventually.
adrocknaphobia commentedon Mar 11, 2013
I'm not convinced this should get added to the backlog (yet). It seems as if the work to upgrade jQuery is simple enough (replacing any deprecated API calls or implementing the migrate plugin). The real effort would be around testing.
I'd like to propose that we leave this issue open for @TuckerWhitehouse (or others) to work on, but when the pull request is ready and extensive testing is required we will add it to the backlog.
I think we should also upgrade directly to 2.0 (and skip 1.9.1).
TuckerWhitehouse commentedon Mar 11, 2013
I have a branch where i'm making all the changes (mostly just the use of .isResolved, .isRejected, .andSelf and $.browser). I had almost everything done, but left the "Whitespace Normalizer" extension enabled... so 1000's of lines got added to the commits for whitespace :/
Testing was successful for everything (unit, integration, etc) except the Inline Code Editor Extension (not sure why that was failing exactly... the errors had nothing to do with jQuery)
As for version, 1.9.1 and 2.0 both have the same api, but 2.0 is still in beta.
Should have a pull request in the next few hours.
TuckerWhitehouse commentedon Mar 12, 2013
My Branch: https://github.com/TuckerWhitehouse/brackets/compare/update_jQuery
jQuery 2.0.0b2 has some bugs, so we have to use 1.9.1 (for now).
4 ProjectManager tests and 1 WorkingSetView test fail on 2.0.0b2
The Bootstrap plugins use$.browser which is deprecated. Would it be better to polyfill $ .browser, or look into upgrading these plugins to the latest versions which no longer rely on $.browser?
The Jasmine Spec runner fails to load 2 files. (In the Jasmine Window, Right Click, and Choose Show DevTools).
"test/LiveDevelopment/Inspector/Inspector.json" included from "src/LiveDevelopment/Inspector/Inspector.js"
"test/LiveDevelopment/main.less" included from "src/LiveDevelopment/main.js"
Both of these files are included using a $.get, and the root directory is different in the test window.
8 of the Inline Color Editor tests fail (that have nothing to do with jQuery as far as I can see...)
TuckerWhitehouse commentedon Mar 12, 2013
For the Jasmine Spec runner load failures, I had to modify the Inspector.js and main.js files (see previous comment), and replace the $.get's with require's, and that problem is now fixed.
Still seeing 8 failures with the Inline Color Editor tests :/
pthiess commentedon Mar 12, 2013
@adrocknaphobia Ok, this is exactly why we left it open - usually we would close an item after we moved it to Trello. We can pull the card from the icebox whenever you decide to make it a priority.
peterflynn commentedon Apr 17, 2013
Per the comments in #3123, we have this tracked in the backlog now, so I'll close this bug.