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 API for when the build has "truly" ended #3881

Closed
intrnl opened this issue Nov 25, 2020 · 3 comments
Closed

Add API for when the build has "truly" ended #3881

intrnl opened this issue Nov 25, 2020 · 3 comments

Comments

@intrnl
Copy link
Contributor

intrnl commented Nov 25, 2020

Feature Use Case

Currently, there is no way to know when a build has "truly" ended, and that is when there are no further calls to generate or write, how should an external service know when to stop if its needed during build and output generation?

When you run Rollup through CLI with only one output, there is no issue since we can just stop during generateBundle hook, but it becomes problematic when you have two or more outputs or Rollup is being called programmatically via API where output generation can happen anytime. Reopening the service to only serve output generation adds unnecessary overhead when it could just not be closed in the first place

I've thought about exposing a method in the plugin API itself that would let you manually stop its service but I consider that to be awkward to use especially if you have multiple plugins doing the same thing (Not to mention that people have their own way of naming things)

let pluginAInstance = pluginA(...);
let pluginBInstance = pluginB(...);
// ...

let bundle = await rollup(...);

pluginAInstance.api.stop();
pluginBInstance.api.stop();
// ...

Feature Proposal

Add end method to RollupBundle, which when called:

  • runs the end hook in plugins
  • disallows any further calls to generate or write
let bundle = await rollup(...);
await rollup.write(...);

bundle.end();

await rollup.write(...);
// Error: No more calls to `bundle` or `write` allowed

The end hook can act as a sort of cleanup for plugins, running when the build fails but not during watch mode, removing the awkward need to check for it as a way to stop the service early.

Additional notes

  • I don't mind contributing this myself if needed, though I certainly want to hear what others would think about such a feature 👍
  • Every once in a while, rollup-plugin-esbuild kept throwing random mysterious errors saying that the service was somehow closed in the middle of build, I finally narrowed it down to the plugin trying to close the esbuild service during generateBundle and my Rollup config having two outputs
@lukastaegert
Copy link
Member

I like the proposal as in contrast in previous ideas, there is a clear way how to use it in the JS API. The CLI can easily make sure the hook is called for individual runs, and I hope we can also make the watch API respect that.

At that point I am only wondering about the name, bundle.end does not sound quite hitting the spot to me as it is not the bundle that is ended, it just cannot be used any more. Originally you proposed close, which I think looks right when used from the JS API as it is similar to closing a server, but looks weird as a plugin hook. Maybe closeBundle? Alternatively, it could be bundle.close() for the JS API and a closeBundle hook for plugins?

In any case, a PR would definitely be welcome for this, otherwise I will hope to have a look once I am done with other things I am working on at the moment.

One danger I see is that there is no mechanism that enforces users to use this, so it needs to be well-documented in the JS API section that this is something one needs to do now. On the other hand, I do not think this is a breaking change for Rollup to add this. But it will be a breaking change for plugins to support this as they can then only be used with the CLI and in environments that explicitly call this.

@intrnl
Copy link
Contributor Author

intrnl commented Nov 26, 2020

I think closeBundle for the hook fits better, definitely had a hard time choosing the name as they don't seem to be fitting 😆

@intrnl intrnl mentioned this issue Nov 26, 2020
15 tasks
@intrnl
Copy link
Contributor Author

intrnl commented Dec 14, 2020

#3883 has been merged now, closing.

@intrnl intrnl closed this as completed Dec 14, 2020
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

2 participants