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

Add sequential before build hook #2826

Open
vladshcherbin opened this issue Apr 30, 2019 · 17 comments
Open

Add sequential before build hook #2826

vladshcherbin opened this issue Apr 30, 2019 · 17 comments

Comments

@vladshcherbin
Copy link
Contributor

Hey, 👋

Feature Use Case

Imagine 2 plugins: delete (deletes dist folder), copy (copies files in dist folder).

They need to be executed before build. Current buildStart hook seems like a good choice, but it runs in parallel. Delete plugin must run before copy and I can't find any other suitable hook for it.

Feature Proposal

Add async, sequential hook which runs before build or make possible buildStart hook to execute some plugins sequentially.

@vladshcherbin
Copy link
Contributor Author

@guybedford, you've added buildStart hook in #2208, maybe you have an idea on how this can be solved? 🙏

@lukastaegert
Copy link
Member

Tricky. We could always just make the buildStart hook sequential but this could make everything unnecessarily slow in case different plugins do some unrelated slow tasks in this hook. The same would apply to a beforeBuild hook as well.

Any reason why the copying needs to happen at buildStart and not at a later time? Actually I would have seen renderStart as the proper hook to delete stuff and generateBundle or writeBundle as the hook where I would do copying.

@lukastaegert
Copy link
Member

As an easter egg at the moment (official API hopefully upcoming), you can also add arbitrary files to a build without the need of a file operation by using generateBundle and adding stuff like

generateBundle(options, bundle) {
  bundle['file.ext'] = {fileName: 'file.ext', isAsset: true, source: 'This is the content'}
}

@vladshcherbin
Copy link
Contributor Author

@lukastaegert I find it intuitive to do tasks not related to bundling in separate from bundling hooks, to kinda have a separation from bundling and other tasks. So, in my head the flow is - delete files, copy files, then generate bundle and write it.

I actually tried to use generateBundle to copy stuff (as it has outputOptions access), but it executes plugin multiple times when output is an array.

I feel like I'm trying to hack hooks, which were designed to be part of bundling and not for doing outside tasks :)

@vladshcherbin
Copy link
Contributor Author

renderStart for delete will also execute multiple times when output is an array of targets.

I have a small test with hooks, chunks and multiple targets, it gives me this:

Hook order output screenshot

Screen Shot 2019-05-02 at 20 25 02

I feel like a global before and after hooks (on top and bottom of this list) would be a nice addition and already have use cases.

What do you think?

@lukastaegert
Copy link
Member

Unfortunately this is conceptually not possible. Rollup CLI is just a thin wrapper around Rollup's JavaScript API which does not know when you are finished calling generate over and over again for each output. And there ARE before and after hooks around all parts which the JavaScript API can control:

  • buildStart before anything has happened during the build phase (options does not really count because it does not have context functions available as options are not known before that hook)
  • buildEnd once the last part of the build phase is completed
  • renderStart before each output is rendered
  • generateBundle/writeBundle once an output has been successfully generated
  • renderError if generation failed so that you can do clean-up if necessary

So going from there, I would put logic that is relevant for all outputs such as cleaning the dist folder into buildStart or buildEnd (the latter might make even more sense as then you will still have the previous bundle available if the build phase failed). Logic that is specific to a single output (such as copying a file into the folder for that output) could go into either renderStart or generateBundle/writeBundle.

@vladshcherbin
Copy link
Contributor Author

@lukastaegert okay, so delete functionality can go to buildStart or buildEnd - this is perfectly fine.

For copying, imagine this example:
you generate 2 bundles, one for commonjs and one for module. The output options will be:

output: [
  {
    file: 'dist/file.commonjs.js',
    format: 'commonjs'
  },
  {
    file: 'dist/file.module.js',
    format: 'module'
  }
]

So, they are both generated to the same place, copy here is expected to work as a global copy (not per single output), same as a delete. renderStart/generateBundle/writeBundle all run per single output, so if I put copy there, they will be executed multiple times. Since copy is supposed to be treated as a global here, I'd try to use same buildStart/buildEnd hooks, but both of them run in parallel.

This is the reason and use case I opened this issue.

@lukastaegert
Copy link
Member

I understand that, but I do not know what a good solution would be, as the problem is performance vs ordering, which is why I wanted to point out existing alternatives for ordering two plugins, e.g. putting one in buildStart and another in buildEnd.

@vladshcherbin
Copy link
Contributor Author

@lukastaegert yep, this is what I ended with, one in buildStart and one in buildEnd as a current workaround.

@shellscape
Copy link
Contributor

@vladshcherbin given the working workaround, shall we close this one?

@vladshcherbin
Copy link
Contributor Author

@shellscape I'm actually not sure. When I develop a plugin, I definitely feel the limitations of current plugin system, like this one where I have to choose hooks carefully because can't control the order of plugins execution or sync/async plugin execution. I'm still not happy with above workaround but it kinda works for now.

This is related to another issue I subscribed to - #1148. So the main issue is not this one, it's just 1 of many which add difficulties in plugin creation. I feel like plugin system is better suited for plugins which target bundled files and plugins which target other tasks like copying need to use workarounds.

Feel free to close this one if you feel it needs to, I just don't know where to subscribe or what issue to follow to see changes in plugin hooks. A lot of related issues were recently closed but no solution to any of them was found or added. 😢

@shellscape
Copy link
Contributor

I understand. We closed a bunch in the hopes that the authors or participants would speak up, or step up to help because we are short on help these days. I think we should keep this open and track it though given your follow up comments. 👍

@pkit
Copy link

pkit commented Feb 25, 2020

There's currently no way to alter options in an async way.
For example: check if module exists in unpkg or other CDN.
Need to rely on exclude/blacklist, which is ugly.

@lukastaegert
Copy link
Member

This should go into #3397

@shellscape
Copy link
Contributor

@lukastaegert I think we're still feeling this in rollup/plugins#203 (comment). Any suggestions for how to resolve that one?

@dsanders1234
Copy link

dsanders1234 commented Apr 8, 2022

One possible solution might be to have an extra option in addition to plugins option:

plugins: [...],
enforceBeforeBuildPluginOrder: [ 'cleanDist', ['autoGenerateSrc', 'typescript'] ],

  1. Subarrays dictate sub-order (which can start asynchronously with subsequent items in the main array). In other words, although 'typescript' must run after 'autoGenerateSrc,' the other plugins can run async with this duo. However, the other plugins and this duo must run after 'cleanDist'. Any items following a sub-array is run in parallel with it, but items in sub-arrays are run sequentially following the same rules.

To allow plugins to run after all other plugins, we could introduce a special string 'other-plugins', which represent the other plugins (that all run async with each other):

enforceBeforeBuildPluginOrder: [ 'cleanDist', 'other-plugins', 'autoGenerateSrc', 'typescript'],

It the above example, the 'other-plugins' start after cleanDist, and all must finish before 'autoGenerateSrc' starts, which is then followed by 'typescript'.

@benmccann
Copy link
Contributor

benmccann commented Jul 27, 2022

We have the same problem, but at the end of the build. We need to run three plugins in order at the end of our build, but don't have a way to do so. We can use writeBundle for one and closeBundle for a second and then we're out of luck. We need a sequential hook at the end of the build.

As an alternative to adding an additional sequential hook, I wonder if it would be worth trying to come up with a way to opt some plugin hooks out of running in parallel. E.g. if I name my hook seqCloseBundle then it would run sequentially or it could be something like:

    closeBundle: {
      execution: 'sequential',
      async handle() {
        // ...
      }
    }

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

No branches or pull requests

6 participants