feat(html): support async template function in module.parser.html.template#21098
feat(html): support async template function in module.parser.html.template#21098aryanraj45 wants to merge 2 commits into
Conversation
…plate Converts the per-module processResult hook (SyncWaterfallHook → AsyncSeriesWaterfallHook) so the html template option can return a Promise<string>. Existing sync tappers (CSS, JS, TS) are unaffected.
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for asynchronous HTML template functions in the HTML parser pipeline by making the template application and the NormalModule processResult hook async-aware.
Changes:
- Allow
module.parser.html.templateto return aPromise<string>in types and implementation. - Convert
NormalModuleCompilationHooks.processResultfrom sync to async-waterfall and update call sites. - Add a new config-case test covering async template behavior and warning emission.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| types.d.ts | Updates typings for async HTML template and async processResult hook. |
| lib/html/HtmlParser.js | Implements Promise support for template / applyTemplate. |
| lib/html/HtmlModulesPlugin.js | Awaits applyTemplate via processResult.tapPromise. |
| lib/NormalModule.js | Migrates processResult hook to AsyncSeriesWaterfallHook and uses callAsync. |
| test/configCases/html/parser-template-async/* | Adds a new test case and warning snapshot for async templates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (typeof transformed === "string" || Buffer.isBuffer(transformed)) { | ||
| return transformed; | ||
| } | ||
| if (transformed instanceof Promise) { | ||
| return transformed.then((result) => { | ||
| if (typeof result !== "string") { | ||
| throw new Error( | ||
| "The `template` html parser option must return a string or Promise<string>." | ||
| ); | ||
| } | ||
| return result; | ||
| }); | ||
| } | ||
| return transformed; | ||
| throw new Error( | ||
| "The `template` html parser option must return a string or Promise<string>." | ||
| ); |
| if (typeof transformed === "string" || Buffer.isBuffer(transformed)) { | ||
| return transformed; | ||
| } | ||
| if (transformed instanceof Promise) { |
… path Aligns sync and async template return type: both now accept only string / Promise<string>. Buffer was silently accepted by the old sync path but was never a valid use case for a text-transformation function.
alexander-akait
left a comment
There was a problem hiding this comment.
Don't touch this things right now, we need to make parser and generator async firstly, we already have a PR for generator, so async is postpone currently
Got it lets wait for the parser/generator async foundation to be in place; will revisit and rebase after that |
Summary
The
module.parser.html.templateoption only supported synchronous functions. This addsPromise<string>return support, closing the html-loaderpreprocessorasync parity gap.Implemented by converting the per-module
processResulthook fromSyncWaterfallHooktoAsyncSeriesWaterfallHook. Existing sync tappers (CSS, JS, TypeScript) are unaffected —.tap()callbacks remain valid on an async waterfall hook. A minor per-module async overhead applies to all module types; benchmarks should be checked.What kind of change does this PR introduce?
feat
Did you add tests for your changes?
Yes —
test/configCases/html/parser-template-async/. Covers async URL injection,addDependency, andemitWarningcalled afterawait.Does this PR introduce a breaking change?
No. Sync templates continue to work unchanged.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
module.parser.html.templatedocs should note it now acceptsasyncfunctions /Promise<string>.Use of AI
partial