search

jquery / jquery-migrate

A development tool to help migrate away from APIs and features that have been or will be removed from jQuery core
Other
2.03k stars 470 forks source link

Differentiate between compatibility patches and pure warnings #472

Open mgol opened 2 years ago

mgol commented 2 years ago

jQuery Migrate does two things:

  1. Fills in removed APIs, warning when used.
  2. Warns about deprecated APIs that still exist.

It makes sense to differentiate these two for a few reasons:

  1. Teams may not always have the capacity to tackle all warnings, they may want to resolve just the ones that block Migrate removal and then remove Migrate, leaving resolving the other warnings for later, when they prepare for updating to the next major version.
  2. A project may have already been on jQuery 3.x without Migrate and then it added Migrate to prepare for an update to jQuery 4.0. But Migrate would now also restore some APIs & behaviors removed in jQuery 3.0 which is counterproductive. Since gh-450, it's now possible to exclude warnings but this use case would require first determining those patches.

We should have an easier way to only enable patches required to address before removing Migrate or to only enable the ones preparing for the next major version.

Thoughts, @dmethvin @timmywil @gibson042?

dmethvin commented 2 years ago

I agree we're doing two different things here, but wonder what the solution is for this. Would we create another flag like jQuery.disableDeprecationWarnings or something like that?

mgol commented 2 years ago

Yeah, naming is hard. ;) Would a single API be enough, considering there are use cases both to only enable patches required to make the current major version of jQuery work post-upgrade and to only enable deprecation warnings for a future major?

We could also bake it into jQuery.migrateDisablePatches & jQuery.migrateEnablePatches as some special keys like compatibility-patches & deprecation-patches (again, better naming suggestions welcome). Together with that, we could introduce a special key all that would disable all patches in case someone wants to enable one by one instead of disabling one by one.

Then we'd just need to keep a list of compat patches and compare those special keys against that. Thoughts?

mgol commented 4 weeks ago

I've been thinking about this more. From my original list, the first point - about differentiating between warnings for APIs that still exist and patches - seem less important to me than the second one - about not allowing patches restoring behavior from earlier jQuery versions than the one from which the upgrade process starts. Let's focus on that.

This is important as Migrate will now support both jQuery 3.x & 4.x and we don't want people to restore the 1.9 behavior when trying to upgrade from 3.x to 4.x.

I think we need to be able to specify the jQuery major from which the update starts, considering that breaking changes should generally only land in major releases now (jQuery 3.5 was a one-off BC due to a security issue). An API like:

jQuery.migrateUpgradingFromJQuery( major );

where major can be 1, 2, 3 or 4. If set to 1 or 2, all patches would be enabled. If set to 3, only the ones covering APIs removed in 4.0.0 or later. If set to 4, only the APIs that still exist would be patched to provide warnings.

I'd also propose for jQuery Migrate 3.x to have it set to 1 by default, and for jQuery Migrate 4.x - to have it set to 3, so that the default patching behavior is restoring less legacy cruft.

We also need to think how to implement that - we need to somehow associate each warning with a jQuery version where a related breaking change occurred - or none, if the warning is for an API that still exists in all stable jQuery versions. It doesn't seem clean to repeat the major in each migrateWarn call, so we should have a way to register this metadata in some kind of a central registry. I don't want to keep it in a single place, though, so that we don't end up with legacy registrations for warnings already removed. Perhaps we need an internal API like registerWarningForMajor( major ) where major can be 3, 4 or undefined - the last one for APIs that still exist. We should also incorporate a check into migrateWarn that a registration was already done, or throw an error.

If someone has a better idea for how to implement this, or what a good API would look like here, I'm all ears!

On a related note, I'm also considering making Migrate 4.x only support running with jQuery 3.5.0 or newer. jQuery 3.5.0 was released 4.5 years ago, it has no reported security issues and is a breaking change release despite being a minor update. It'd also make our tests quicker due to a smaller set of tested jQuery versions.

timmywil commented 2 weeks ago

Discussed in meetings and landed on the following: