Allow using objects as hooks to change execution order

[lukastaegert]

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

List any relevant issue numbers:
See https://github.com/vitejs/vite/discussions/9442 for context

Description

This PR will be released in a few days if there are no further comments.

This PR implements the following:

• For each hook, you can supply an object with a handler property instead of a function:

  const plugin = {
    buildStart: {
      handler() {
        // actual implementation
      }
    }
  }

• For banner, footer, intro, outro, the handler can also be a string.
• Furthermore, you can add a property order?: 'pre' | 'post' | null that changes the execution order for this hook (similar to what Vite's enforce currently does for all hooks):

  const plugin = {
    resolveId: {
      // Run before all other resolveId hooks that do not have 'pre';
      // hooks with the same value are sorted by plugin order
      order: 'pre',
      handler(source, importer) {
        // actual implementation
      }
    }
  }

• For parallel hooks, this will still change the execution order of the synchronous part.
• For parallel async hooks, you can also specify sequential: true. When such a hook is encountered, all previous parallel plugin hooks will be awaited. Then the sequential hook will be executed and awaited before continuing with the next plugin. If none of them is sequential, the remaining plugin hooks will be executed in parallel again. This can be combined with order.

Why not enforce at the top level?

While the top level may be enough for simple plugins that only have a single hook, the problem of plugin ordering is not a problem of ordering all hooks, it is usually only the problem of ordering a single hook. A good example is the commonjs plugin: While it would want its transform hook to be executed at the user-provided order, the resolveId hook needs to be executed before other plugins as it is needed to detect entry points (currently this is solved by injecting a secondary plugin in the options hook, but this is probably not ideal).

As it stands, my plan would be not to support top-level enforce at all but go with something like this. As this was born from discussions with the Vite people, they would likely support it as well if we align on all details.

Why the object form?

At various times it was suggested to add certain preSomething or postSomething hooks. I do not like the idea as they are just further polluting the number of available hooks and are themselves rather inflexible. With the proposed implementation, we can further refine things by allowing additional values or additional parameters on the object. Also, the original discussion started from the question if we cannot make certain parallel hooks sequential on demand. This could be added with another object property as well. But I decided to go for execution order first as it has more immediate use for Rollup itself.

Why support it for all async hooks, not just non-parallel ones?

The goal is also to support making parallel hooks sequential. Before this is implemented, the object form does not hurt.

[github-actions]

Thank you for your contribution! ❤️

You can try out this pull request locally by installing Rollup via

npm install rollup/rollup#object-hooks

or load it into the REPL:
https://rollupjs.org/repl/?pr=4600

[codecov]

Codecov Report

Merging #4600 (fd3d91f) into master (1165d46) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4600   +/-   ##
=======================================
  Coverage   98.86%   98.87%           
=======================================
  Files         209      209           
  Lines        7345     7366   +21     
  Branches     2098     2102    +4     
=======================================
+ Hits         7262     7283   +21     
  Misses         27       27           
  Partials       56       56           

Impacted Files	Coverage Δ
src/rollup/rollup.ts	100.00% <100.00%> (ø)
src/utils/PluginContext.ts	100.00% <100.00%> (ø)
src/utils/PluginDriver.ts	100.00% <100.00%> (ø)
src/utils/error.ts	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[antfu]

This is awesome! Thank you for working on it! 💚

Should we also support the object form for banner/footer/intro/outro

In my personal opinion, I'd suggest that we don't do it now, but until there are valid cases for them. Deps on the use cases, we might come up with other options to add.

Is handle good?

handler sounds good to me if we want to change.

How about enforceOrder?

I agree enforce would be a bit too general (I guess it's borrowed from Webpack). IMO order would be better than enforce if we want to a shorter name. And I think your pick enforceOrder is great for both explicitness and semantics.

[patak-dev]

Thanks for working on this @lukastaegert!

About naming, same as Anthony, enforceOrder and handle (or handler) sounds good to me too. Maybe all the names should be imperative or not inside the object form? For later:

• A. { handle, enforceOrder, execute }
• B. { handler, order, execution }

I think I prefer B here, but both look fine.

If the hook is not parallel (i.e. sequential or first), then you can add a property enforceOrder?: 'pre' | 'post' | null that changes the execution order for this hook

Using enforceOrder in parallel hooks still makes sense, so I think it should be added for parallel hooks at this point too. Even if the hook is executed in parallel, being able to control the order lets you kick in synchronization between the hooks (this is what we were suggesting people do before). Once there is a new execution option, these two would be orthogonal and could be used together (at least in the design proposed by https://github.com/vitejs/vite/discussions/9442)

[lukastaegert]

{ handle, enforceOrder, execute }

Stupid question. but what would be the purpose of "execute"?

[patak-dev]

execute is the imperative of execution: 'sequential' | 'parallel' as described in https://github.com/vitejs/vite/discussions/9442. I think the non-imperative naming is less confusing for the hooks object

[lukastaegert]

Ah, of course you are right. Not sure we can (or should) enforce parallel execution of non-parallel hooks. E.g. transform and renderChunk need to be sequential as they are transforming a value. So maybe rather executeSequentially, and just for parallel hooks?

For the non-imperative form, it could just be sequential: boolean

[lukastaegert]

At this point, the non-imperative form is beginning to look much more concise.

[antfu]

Love sequential: boolean 👍

[lukastaegert]

Even if the hook is executed in parallel, being able to control the order lets you kick in synchronization between the hooks

Good point, even parallel hooks have a synchronous part that is executed in a specific order.

[benmccann]

This looks pretty good to me 😄 Would it make sense to update the PR description with the latest changes? I'm not 100% sure I have a full understanding of the current state of things

[lukastaegert]

Done. To sum it up so far I

• changed the approach to handler and order
• added support for object hooks and order to all async hooks, including banner/footer/intro/outro and parallel hooks.

I still did not get to implementing "sequential", and I am also considering extending support to sync hooks as well.

[patak-dev]

I still did not get to implementing "sequential", and I am also considering extending support to sync hooks as well.

+1 to extend the object form to sync hooks too. The order modifier should be useful on sync hooks too (in Vite, enforce affects every hook as it changes the plugin position directly). I think it would be also good for teaching/docs if the object form works across the board. Later additions like sequential may only affect parallel hooks, but at least the { handler, order } (and maybe later other modifiers like filter, not proposing it, just to illustrate) would be valid regardless of the hook type.

[lukastaegert]

Sync hooks are now supported as well.

[lukastaegert]

And finally, sequential is implemented as well. At this point, only documentation would be missing. sequential can currently be added for all parallel hooks except banner/footer/intro/outro as those always need some special handling due to the fact that they can have string values. I could extend to those as well, or we leave it as it is.

For parallel async hooks except banner, footer, intro, outro, you can also specify sequential: true. All plugins that specify that flag will be run sequentially once all parallel plugins are done. This can be combined with order, which will sort the parallel and sequential plugins separately.

[patak-dev]

About the execution order of parallel hooks when sequential is present, we discussed in the Vite RFC that is better to make this flag orthogonal to the order. Hooks with sequential will await the execution of previous hooks before being called. See comment here https://github.com/vitejs/vite/discussions/9442#discussioncomment-3284799, @antfu updated the RFC after that thread.

[benmccann]

@patak-dev was there a usecase you had in mind for that behavior? @antfu's original proposal that's implemented here made sense to me because traditionally if a hook is parallel then it means you don't care about the order it's run in. I'm not opposed to the behavior you suggested, but it's not what I originally envisioned and am not quite sure when it would be necessary

[lukastaegert]

@patak-dev I must admit I must have slightly skipped over your comment😳 Yes, I can also implement it the other way. I am just very happy I decided to write documentation last 😅

[patak-dev]

@benmccann

@antfu's original proposal that's implemented here made sense to me because traditionally if a hook is parallel then it means you don't care about the order it's run in.

The order of parallel hooks still matters. Each hook is executed in order and all the generated promises are then awaited:

await Promise.all( hooks.map( h => h(...) )

The order of the sync part of each hook could allow a user to introduce custom synchronization even without the sequential flag. For example, a plugin may expose a waitFor API that previous hooks could call to register themselves. Or a hook may start a synchronization scheme that needs to be executed before all others. This isn't ergonomic, and the complexity to leave this to users is in part what justifies to me having the sequential flag in Rollup.
Even something simpler like logging that a hook started currently depends on rollup executing parallel hooks in order. What doesn't matter is the order in which these hooks finish to execute.

So, I think it should be the other way around. We should be looking for a usecase to justify sequential to both affect the way a hook is awaited and its position in the pipeline. The usecases we discussed so far don't require we overload this flag in such a way. It is harder to teach the flag if it not only makes a particular hook sequential, but also pushes it to the end of all parallel hooks.

We can let sequential affect the hook in place, and be orthogonal to the hook position in the pipeline. The position of the hook would be controlled as usual by the position of the plugin in the pipeline, and we now have order if you would like to push hooks before or after their normal place.

You can get the effect of the current implementation with a single plugin by using { handler, sequential: true, order: 'post' }, or by using { handler, sequential: true } and position the plugin after all other parallel plugins.
On the other hand, the current implementation is less flexible than the proposed orthogonal approach. For example, you may want to have a sequential hook before all the parallel hooks, and even when using order 'pre', you won't be able to get that.

The implementation complexity is the same at this point, but I think the orthogonal flag will better play with other options we could add in the future. Each of them having a single effect is easier to teach and will end up allowing us to cover more use cases.

[lukastaegert]

I have changed the logic as suggested by @patak-dev and also extended it to include banner/footer/intro/outro

For parallel async hooks, you can also specify sequential: true. When such a hook is encountered, all previous parallel plugin hooks will be awaited. Then the sequential hook will be executed and awaited before continuing with the next plugin. If none of them is sequential, the remaining plugin hooks will be executed in parallel again. This can be combined with order.

At this point, only documentation is missing, then this can be released.

[lukastaegert]

Documentation has been added as well. I will leave this PR open for a few days and then release it if there are no further comments.

Documentation

Instead of a function, hooks can also be objects. In that case, the actual hook function (or value for banner/footer/intro/outro) must be specified as handler. This allows you to provide additional optional properties that change hook execution:

• order: "pre" | "post" | null
  If there are several plugins implementing this hook, either run this plugin first ("pre"), last ("post"), or in the user-specified position (no value or null).

  export default function resolveFirst() {
    return {
      name: 'resolve-first',
      resolveId: {
        order: 'pre',
        handler(source) {
          if (source === 'external') {
            return { id: source, external: true };
          }
          return null;
        }
      }
    };
  }

  If several plugins use "pre" or "post", Rollup runs them in the user-specified order. This option can be used for all plugin hooks. For parallel hooks, it changes the order in which the synchronous part of the hook is run.

• sequential: boolean
  Do not run this hook in parallel with the same hook of other plugins. Can only be used for parallel hooks. Using this option will make Rollup await the results of all previous plugins, then execute the plugin hook, and then run the remaining plugins in parallel again. E.g. when you have plugins A, B, C, D, E that all implement the same parallel hook and the middle plugin C has sequential: true, then Rollup will first run A + B in parallel, then C on its own, then D + E in parallel.

  This can be useful when you need to run several command line tools in different writeBundle hooks that depend on each other (note that if possible, it is recommended to add/remove files in the sequential generateBundle hook, though, which is faster, works with pure in-memory builds and permits other in-memory build plugins to see the files). You can combine this option with order for additional sorting.

  import { resolve } from 'node:path';
  import { readdir } from 'node:fs/promises';
  
  export default function getFilesOnDisk() {
    return {
      name: 'getFilesOnDisk',
      writeBundle: {
        sequential: true,
        order: 'post',
        async handler({ dir }) {
          const topLevelFiles = await readdir(resolve(dir));
          console.log(topLevelFiles);
        }
      }
    };
  }

[ElMassimo]

Created an RFC, but I'll drop a note so that we can have it mind for the design of the order option:

• https://github.com/vitejs/vite/discussions/9633

---

Some plugins need to run before or after a specific plugin (i.e. before or after the Vue or Svelte compiler transforms).

Currently, Vite and Rollup do not provide an explicit API or plugins to collaborate with each other explicitly.

As a result, userland plugins are resorting to hacks and workarounds to achieve a better UX.

Extending order to support { pre?:string, post?: string }, a plugin like Vuetify's could ensure that it always works as expected.

{
  name: 'vuetify:import',
  transform: {
    order: { post: 'vite:vue' },
    async handler (code, id) {
      ...
    },
  },
}

This would enable better compatibility between plugins that interact or extend each other, and result in a better user experience by removing the need to order plugins manually (in cases like these).

[patak-dev]

I discussed with @ElMassimo his proposal, and @brillout proposed something similar in the linked Vite RFC. I was originally against adding more complex ordering, but now I think it is something worth exploring. Mainly because frameworks are already taking control and reordering the plugins in different ways, leading to potentially more confusion.

One of the things I like about the hooks object form is that it will allow us to do extensions if needed in the future. So, I think we should move forward with this PR as is at this point.

@ElMassimo, @brillout, I think that the best place to gather consensus about further ordering mechanisms is by starting a discussion on the Vite repo and involving the main frameworks.

[brillout]

Sounds good to me 👌.

Just opened a new discussion for this: vitejs/vite - #9613 - Hook ordering.

@ElMassimo the discussion patak mentioned: vitejs/vite - #9442 - comment @brillout.

@lukastaegert I think Rollup users would equally benefit from this. That's why I'm thinking it could/(should?) be a feature in Rollup land.

[antfu]

Link for reference, here is the Vite PR for supporting object hooks on top of this PR

• feat: support object style hooks vitejs/vite#9634

As a side note: imagine this PR gets shipped in Rollup v2.78.0 or something, it will break the types for the current versions of Vite until the Vite PR gets merged. I didn't see a straightforward way to make it smoother so I guess it would be better to work closely on both sides and make the gap as narrower as possible.

[lukastaegert]

I didn't see a straightforward way to make it smoother

Would it be worthwhile to go for an intermediate Vite release where you use something like Hook extends Function ? Hook : never to guard against object hooks, or should we just coordinate releases close together?

[patak-dev]

@lukastaegert we narrowed down the rollup version to 2.77 in vite 3.0.6 yesterday. So if you release this in 2.78, it should not affect Vite.

[patak-dev]

Oh, we should also backport the narrowing down to Vite 2.9. I'll do that today

Done, vite@2.9.15 is ready too

[lukastaegert]

Great work! If nothing else comes up, I plan on releasing this about this time tomorrow.

[lukastaegert]

Released in v2.78.0