Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Upgrade jQuery #3072

Description

@TuckerWhitehouse

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

  1. peterflynn commented on Mar 8, 2013

    @peterflynn
    Member

    @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.

  2. TuckerWhitehouse commented on Mar 8, 2013

    @TuckerWhitehouse
    ContributorAuthor

    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

    • editor/CodeHintManager.js (line 344)
    • utils/NodeConnection.js (line73)
    • LiveDevelopment/Inspector/Inspector.js (line 269)
    • extensions/default/HTMLCodeHints/main.js (lines 290, 299 & 305)

    I can go through and start replacing these and submit a pull request

  3. pthiess commented on Mar 11, 2013

    @pthiess
    Contributor

    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.

  4. adrocknaphobia commented on Mar 11, 2013

    @adrocknaphobia
    Contributor

    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).

  5. TuckerWhitehouse commented on Mar 11, 2013

    @TuckerWhitehouse
    ContributorAuthor

    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.

  6. TuckerWhitehouse commented on Mar 12, 2013

    @TuckerWhitehouse
    ContributorAuthor

    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...)

  7. TuckerWhitehouse commented on Mar 12, 2013

    @TuckerWhitehouse
    ContributorAuthor

    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 :/

  8. pthiess commented on Mar 12, 2013

    @pthiess
    Contributor

    @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.

  9. peterflynn commented on Apr 17, 2013

    @peterflynn
    Member

    Per the comments in #3123, we have this tracked in the backlog now, so I'll close this bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions