feat: support async plugins #4671
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4671 +/- ##
==========================================
- Coverage 99.05% 99.02% -0.04%
==========================================
Files 214 215 +1
Lines 7548 7552 +4
Branches 2092 2092
==========================================
+ Hits 7477 7478 +1
- Misses 23 24 +1
- Partials 48 50 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Looks great already. Some minor points I found and of course we again need to document it a little.
I keep thinking about the coverage change in watch-cli. I do not think it is something to worry about in your ticket. In the end, we should probably send an exit code 1 to close in the uncaughtException handler.
src/rollup/types.d.ts
Outdated
| | false | ||
| | undefined | ||
| | InputPluginOption[] | ||
| | Promise<Plugin | false | null | undefined | InputPluginOption[]>; |
There was a problem hiding this comment.
As a mini optimization so we do not need to repeat everything, maybe use either an intermediate type so that
type InputPluginOption = InputPluginOptionValue | Promise<InputOptionValue>or introduce a utility type like
type MaybePromise<T> = T | Promise<T>;that could also be reused in other places?
There was a problem hiding this comment.
Done. I'll make more PR to improve and simplify types.
| array = (await Promise.all(array)).flat(Infinity) as any; | ||
| } while (array.some((v: any) => v?.then)); | ||
| return array; | ||
| } |
There was a problem hiding this comment.
I wonder it would not make sense to integrate the filter(Boolean) here as well so you do not need the optional chaining in v?.then? But then maybe this should just become normalizePluginOption?
There was a problem hiding this comment.
This function is copied from Vite.
I think we just keep the function simple and keep the falsy value (the fn is not referenced by now, but in the future, it could be used in other places). filter(Boolean) is done in normalizePluginOption.
|
I didn't get your point of Are there any chances to emit rollup/src/watch/watch-proxy.ts Lines 12 to 14 in bc88510 Line 31 in bc88510 |
|
This PR has been released as part of rollup@3.2.0. You can test it via |
Sorry, missed that last question. Yes, it is handled, but it does process.on('uncaughtException', close);
async function close(code: number | null): Promise<void> {
process.removeListener('uncaughtException', close);
// removing a non-existent listener is a no-op
process.stdin.removeListener('end', close);
if (watcher) await watcher.close();
if (configWatcher) configWatcher.close();
if (code) {
// eslint-disable-next-line unicorn/no-process-exit
process.exit(code);
}
}which means no explicit exit code is passed to close, instead it passes the event as code. That means |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
Add async plugin support
Do not merge this PR before upstream PR #4668 gets merged