Calling this.addWatchFile in load hook throws an error if manualChunks is used

[sapphi-red]

I'm not sure if this is a bug. In the documentation, this.addWatchFile is allowed to be called in the build phase, i.e. load hook.

This context function can only be used in hooks during the build phase, i.e. in buildStart, load, resolveId, and transform.
https://rollupjs.org/plugin-development/#this-addwatchfile

But actually load hook is called in the generate phase (e.g. when manualChunks is used (addAdditionalModules)). So calling this.addWatchFile unconditionally will throw an error in that case. The error happens with both 4.5.0 and 3.29.4.

Reproduction: https://stackblitz.com/edit/rollup-repro-zyzeva?file=rollup.config.js

Should I add a if condition like this?

let isGeneratePhase = false
const plugin = {
  name: 'load',
  async load(id) {
    const content = await fsp.readFile(id, 'utf-8');
    !isGeneratePhase && this.addWatchFile(id);
    return content;
  },
  buildEnd() {
    isGeneratePhase = true
  }
}

[lukastaegert]

Yes I see. I fear this is a tricky edge case that was not properly considered for manual chunks. The problem is that watchFiles that are added during the generate phase would be ignored. We could avoid throwing an error as it would also not be a problem, I am just not sure we should. Maybe we could avoid throwing an error if they are added in a hook that usually runs in the build phase, or at least for the load hook.

[sapphi-red]

Avoid throwing an error if they are added in a hook that usually runs in the build phase.

I think this is safe to do and this matches my intuition the most. Because IIUC other build phase hooks (e.g. resolveId) will be also called from addAdditionalModules.

The only problem I can think of is this: when

• running in watch mode
• a non-imported module (foo.js) is included in manualChunks
• a plugin calls this.addWatchFile('bar.js') in transform hook when transforming that module

, will editing bar.js correctly trigger a new build?

[lukastaegert]

Probably not. On the other hand looking at the code, I think there is no reason not to update watchFiles in the CLI AFTER the generate phase. That would remove this restriction entirely. Might be another two days before I can look into this, though.

[lukastaegert]

Fix at #5270

[github-actions]

This issue has been resolved via #5270 as part of rollup@4.6.0. You can test it via npm install rollup.