search

mozilla / addons

☂ Umbrella repository for Mozilla Addons ✨
Other
128 stars 41 forks source link

upgrade jquery to 3.2.* #4471

Closed eviljeff closed 7 years ago

eviljeff commented 7 years ago

We're on 1.12.0 which is unsupported now so we should upgrade to the latest version.

https://jquery.com/upgrade-guide/3.0/

eviljeff commented 7 years ago

jquery.pjax is triggering an error about jQuery.event.props but seems to be an issue that can be ignored, at least according to the maintainer

eviljeff commented 7 years ago

jQuery.expr[":"] is now jQuery.expr.pseudos is a warning originating from a jqueryui module but it's only deprecated and I assume jqueryui will fix before it gets removed in a later jquery version.

muffinresearch commented 7 years ago

jquery.pjax is triggering an error about jQuery.event.props but seems to be an issue that can be ignored, at least according to the maintainer

Looks like there's an update that makes jquery-pjax compatible.

It's also worth noting that we'd want to use jquery-migrate as minimally as possible - e.g just to help with the migration.

In the past versions of jquery-migrate have been known to re-introduce security issues for backwards compatibility - whilst that's not necessarily the case this time, it's still a good idea to not leave jquery-migrate in place after the upgrade and additionally restrict it's inclusion so it doesn't make it to production.

eviljeff commented 7 years ago

Looks like there's an update that makes jquery-pjax compatible.

We're using that version, but it still triggers the warnings.

It's also worth noting that we'd want to use jquery-migrate as minimally as possible - e.g just to help with the migration. In the past versions of jquery-migrate have been known to re-introduce security issues for backwards compatibility - whilst that's not necessarily the case this time, it's still a good idea to not leave jquery-migrate in place after the upgrade and additionally restrict it's inclusion so it doesn't make it to production.

I wasn't planning on landing jquery-migrate in a commit, I just made the change locally. I guess we could just include in the debug bundle so it'd only be used locally but I don't think it gives us much extra. I read through the migration document too, and searched for anything that needed changing.

eviljeff commented 7 years ago

QA: watch out for javascript breakage on all non-mobile/new front-end pages. 1.12.4 to 3.2 is a significant upgrade with breaking changes.