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

buildStart hook is not async in webpack #293

Open
brawaru opened this issue Mar 20, 2023 · 4 comments
Open

buildStart hook is not async in webpack #293

brawaru opened this issue Mar 20, 2023 · 4 comments
Labels

Comments

@brawaru
Copy link

brawaru commented Mar 20, 2023

Environment

unplugin 1.3.1
webpack 5.76.2
Node.js v16.14.2

Reproduction

  1. Have buildStart perform async operation.
  2. Have transform hook rely on ordered completion buildStart and its result.

StackBlitz: https://stackblitz.com/edit/webpack-webpack-js-org-1qvdhq?file=package.json&view=editor (pnpm start to run webpack programmatically, pnpm test to run tests which will also do the build programatically).

Describe the bug

buildStart is not an async hook, and invoking await will immediately continue the build pipeline, which will have unexpected consequences.

Vitest:

-- webpack call
-- webpack end

-- buildStart call         ← buildStart waits 1 second to resolve a promise

-- transform call          ← transform called before buildStart is fulfilled
× transform thrown Error: transform method was called either before buildStart call was completed or after it has thrown an error
-- transform end

√ buildStart return        ← timer fires, buildStart fulfills
-- buildStart end

(Test timeout)

Node.js:

-- webpack call
-- webpack end

-- buildStart call

-- transform call
transform call without without buildStart completion, throwing!
× transform thrown Error: transform method was called either before buildStart call was completed or after it has thrown an error
-- transform end

 UNHANDLED EXPECTION 
 Error: transform method was called either before buildStart call was completed or after it has thrown an error

 (Process halted with error code)

Additional context

  • In Rollup/Vite realm buildStart is async and can be used to perform initial initialisation for transform hook invocations.
  • transform hook seems to be correctly async, which means you can await for buildStart fulfillment in it, which involves moving async logic of buildStart to a separate function and then also creating another plugin variable in which you will store promise returned by that separate async function call. But it's not obvious and still feels like a mistake/oversight.

Logs

N/A.
@wChenonly
Copy link

wChenonly commented Jun 12, 2023

It does not seem to be asynchronous, for webpack

I tried to solve it without success

@LorisSigrist
Copy link

The workaround right now is to add webpack specific code to await the promise.

const plugin = createUnplugin(() => {
  return {
      name: "my-plugin",
      webpack(compiler) {
          compiler.hooks.beforeRun.tapPromise("my-plugin", async () => {
              // await Promise in here
          });
      }
}})

@wChenonly
Copy link

The workaround right now is to add webpack specific code to await the promise.

const plugin = createUnplugin(() => {
  return {
      name: "my-plugin",
      webpack(compiler) {
          compiler.hooks.beforeRun.tapPromise("my-plugin", async () => {
              // await Promise in here
          });
      }
}})

yes,At present, this can only be done first.

northernCold added a commit to northernCold/unplugin-tailwindcss-shortener that referenced this issue May 2, 2024
@sxzz sxzz added the webpack label Jul 1, 2024
@SoonIter
Copy link

SoonIter commented Dec 4, 2024

some investigations

the make hook is a AsyncParallel hook in webpack, so loader transform and buildStart is parallel in make stage

compiler.hooks.make.tapPromise(plugin.name, async (compilation) => {
const context = createBuildContext(contextOptionsFromCompilation(compilation), compiler, compilation)
if (plugin.watchChange && (compiler.modifiedFiles || compiler.removedFiles)) {
const promises: Promise<void>[] = []
if (compiler.modifiedFiles) {
compiler.modifiedFiles.forEach(file =>
promises.push(Promise.resolve(plugin.watchChange!.call(context, file, { event: 'update' }))),
)
}
if (compiler.removedFiles) {
compiler.removedFiles.forEach(file =>
promises.push(Promise.resolve(plugin.watchChange!.call(context, file, { event: 'delete' }))),
)
}
await Promise.all(promises)
}
if (plugin.buildStart)
return await plugin.buildStart.call(context)
})
}

this issue will not appear in Rspack, because currently Rspack's make hook is AsyncSeriesHook

https://github.com/web-infra-dev/rspack/blob/4bc1d680fa37c5dd7a40c7a60febd832ea3a03f9/crates/rspack_core/src/compiler/mod.rs#L33

Unfortunately, there is no AsyncSeriesHook with compilation before make

image

thisArg of buildStart is contextOptionsFromCompilation, so there are two ways to solve this issue:

await plugin.buildStart.call(context) // contextOptionsFromCompilation
  1. support context but not support async (now)
  2. support async but not support context: to run buildStart in beforeCompile hook without compilation, so thisArg cannot be passed to buildStart
image

IMO, to comment that buildStart in webpack only supports sync is a good solution, and let users who wants to use async use workaround #293 (comment)

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

No branches or pull requests

5 participants