More flexible hook system #2337
Conversation
|
@sodatea Can I get some feedback on this? |
|
I'm currently working on a patch release. |
|
@sodatea Is this still on the rosdmap in your opinion? |
|
@LinusBorg Yeah I’ve talked to Evan before. He may have some additional ideas on the API design. |
|
I am eager to get this merged. |
packages/@vue/cli/lib/invoke.js
Outdated
| } | ||
|
|
||
| if (afterAnyInvokeCbs.length) { | ||
| logWithSpinner('⚓', `Running completion hooks from other plugins...`) |
There was a problem hiding this comment.
Does this include the afterAnyInvoke hook from the currently running generator?
There was a problem hiding this comment.
From all the current plugins.
There was a problem hiding this comment.
But the order of afterAnyInvoke callbacks is first all other plugins' hook, then the current plugin's afterAnyInvoke hook.
The exact result depends on which plugin did you invoke first.
This looks confusing to me.
It is the same kind of non-determinism like npm install. We should avoid it.
There was a problem hiding this comment.
Also the log is misleading in that case.
There was a problem hiding this comment.
Fixed this. Current plugins will run first and then others. The order is now documented.
There was a problem hiding this comment.
It no longer triggers other plugins' afterAnyInvoke hook as I tested.
I'm also a little bit skeptical about this approach - this is still non-deterministic IMO.
For example, if a user has two plugins with afterAnyInvoke hook, say, eslint and typescript, the order will be:
eslint->typescripton creationeslint->typescriptif user addeslintfirsttypescript->eslintif user addtypescriptfirst
My idea is to keep the order of afterAnyInvoke in sync with that of plugin loading (for now, it is the key enumeration order in package.json) and then figure out a way to specify plugin order in a separate PR. What do you think?
There was a problem hiding this comment.
Good idea. I didn't think of it like that. Made it completely deterministic like how you said. Should be working now.
|
Updated the PR |
|
Hey @pksunkara, thank you for your time and effort spent on this PR, contributions like yours help make Vue better for everyone. Cheers! 💚 |
The bug was introduced in #2337.
Fixes #1754, Fixes #1938
Implements the discussion regargding hooks from https://gist.github.com/sodatea/37bc8c36d7d0c285dfea52fbe8ddaf90