fix: rework runSequentialHandlers plugin ordering
#5751
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In prep for extending the `runSequential` tests, break out the `beforeEach` sections to allow for variation in the tests.
We have a bug where if two plugins A and B, that both use the `hre/created` hook, where B depends on A, then we are seeing B run first then A run second. The ordering tests against the chained run have also been expanded.
The plugin reverse ordering of handlers makes sense for the chained running of hooks but is wrong for the sequential running, where dependent plugins should run before the dependee. We make several moves: - remove the `getHandlers` method from the API as it is only used internally to the hook manager - create equivalents to `getHandlers` with explicit logic for reversing the plugin order for chained running, and keeping the dependency order for sequential.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
kanej
commented
Sep 16, 2024
kanej
commented
Sep 16, 2024
kanej
changed the title
runSequentialHandlers plugin ordering
alcuadrado
reviewed
Sep 16, 2024
We change the ordering of dynamic hooks in run sequential, so that it is in effect the reverse of running chained. Plugins are now run first, with dynamic handlers coming second. Plugins run in resolved dependency order (if B depends on A, A runs before B). Dynamic handlers are run in the order they are registered. The tests and docs have been updated to reflect this change.
alcuadrado
approved these changes
Sep 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The plugin reverse ordering of handlers makes sense for the chained
running of hooks but is wrong for the sequential running, where
dependent plugins should run before the dependee.
We make several moves:
getHandlersmethod from the API as it is only usedinternally to the hook manager
getHandlerswith explicit logic for reversingthe plugin order for chained running, and keeping the dependency order
for sequential.
Reviewers Suggestion
This PR should be reviewed a commit at a time to avoid the jitter from the test restructuring.