Skip to content
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

Differentiate between compatibility patches and pure warnings #472

Open
mgol opened this issue Aug 12, 2022 · 4 comments
Open

Differentiate between compatibility patches and pure warnings #472

mgol opened this issue Aug 12, 2022 · 4 comments
Milestone

Comments

@mgol
Copy link
Member

mgol commented Aug 12, 2022

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 Core: Support selectively disabling Migrate patches #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
Copy link
Member

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
Copy link
Member Author

mgol commented Aug 14, 2022

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
Copy link
Member Author

mgol commented Oct 17, 2024

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.

@mgol mgol added the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Oct 17, 2024
@timmywil timmywil removed the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Oct 28, 2024
@timmywil
Copy link
Member

timmywil commented Oct 28, 2024

Discussed in meetings and landed on the following:

  • migrate 4.x and future versions will differentiate between warning about deprecations and removals as normal, but treat upcoming changes as info and not warnings. 3.x will remain special since that's how it's worked for a long time.
  • keep migrate versions in line with core versions. simplify 4.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants