-
Notifications
You must be signed in to change notification settings - Fork 57
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
fix: show incompatible plugins for major changes as well #5549
base: main
Are you sure you want to change the base?
fix: show incompatible plugins for major changes as well #5549
Conversation
cf11bcd
to
c18589b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have the full context, but I trust that you've verified the edge cases.
compatWarning !== '' && | ||
version !== undefined && | ||
compatibleVersion !== undefined && | ||
// Using only the major version prevents printing this warning message when |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this no longer a problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly this code already dates back my knowledge, but based on the comment I would assume that this "race" condition is super rare.
It might occur still but the worst case in this scenario is that we show a misleading warning in those edge cases.
Where as in the case of the next.js runtime we don't show this warnings as all (as it's a newer major by design).
I think for all new major versions that we release on plugins we still want to show why it did not used the incompatible (new major) version?
e404019
to
fe14b28
Compare
🎉 Thanks for submitting a pull request! 🎉
Summary
Shows the incompatible plugins log for major version changes as well:
This is a small PR to just unblock the current pain. I'm currently working on a larger refactoring of this area to make it cleaner but this will take more time.
in the meanwhile we can at least see why it did not used the new version
For us to review and ship your PR efficiently, please perform the following steps:
we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
something that`s on fire 🔥 (e.g. incident related), you can skip this step.
your code follows our style guide and passes our tests.
A picture of a cute animal (not mandatory, but encouraged)