Wait for bundle input option

[Heerschop]

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

List any relevant issue numbers:

I'm not sure about the testing part, if it is not okay like this, than can you please give me some pointers.

Description

Added the --waitForBundleInput option. The current version of rollup will generate the "Error: Could not resolve entry module" error when the input file or directory does not exists. With this pull request you can use the --waitForBundleInput option so rollup will wait for the input files to be created.

This will make the usage of rollup in conjunction with other tool easier. For example when you start tsc --watch and rollup --watch in parallel. The current version of rollup will generate an error, because the bundle input is not available yet. When using the new --waitForBundleInput option, rollup can just wait for the bundle input file to be available.

Example using rollup when directory or file does not exist

rollup --config --watch

rollup v2.10.2
bundles tmp/main.js → dist/bundle.js...
[!] Error: Could not resolve entry module (tmp/main.js).
Error: Could not resolve entry module (tmp/main.js).
    at error (./node_modules/rollup/dist/shared/rollup.js:161:30)
    at ModuleLoader.loadEntryModule (./node_modules/rollup/dist/shared/rollup.js:17547:16)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async Promise.all (index 0)
    at async Promise.all (index 0)
    at async getCombinedPromise (./node_modules/rollup/dist/shared/rollup.js:17438:13)
    at async ModuleLoader.awaitLoadModulesPromise (./node_modules/rollup/dist/shared/rollup.js:17443:9)
    at async ModuleLoader.addEntryModules (./node_modules/rollup/dist/shared/rollup.js:17339:33)
    at async Promise.all (index 0)
    at async Graph.parseModules (./node_modules/rollup/dist/shared/rollup.js:18548:63)

Example using rollup with the new --waitForBundleInput option

rollup --config --watch --waitForBundleInput

rollup v2.10.5
bundles tmp/main.js → dist/bundle.js...

[2020-05-21 12:02:40] waiting for bundle input tmp/main.js
created dist/bundle.js in 2.8s

[2020-05-21 12:03:08] waiting for changes...

[codecov]

Codecov Report

Merging #3577 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3577   +/-   ##
=======================================
  Coverage   96.35%   96.36%           
=======================================
  Files         180      181    +1     
  Lines        6151     6166   +15     
  Branches     1808     1812    +4     
=======================================
+ Hits         5927     5942   +15     
  Misses        111      111           
  Partials      113      113           

Impacted Files	Coverage Δ
src/utils/options/mergeOptions.ts	100.00% <ø> (ø)
cli/run/commandPlugins.ts	97.05% <100.00%> (+0.18%)	⬆️
cli/run/waitForInput.ts	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2040f82...bab4a4b. Read the comment docs.

[lukastaegert]

Thank you for your PR. I see how this is useful for your setup, however I am not sure if we want to make this a core feature. Reasons are mainly architecturally as I think this feature is only ever useful in watch-mode, but now adds quite a bit of additional churn to the already very crowded module loader. And there will be some maintenance cost in the future as well as this possibly makes some upcoming refactorings harder.

Why not make it a plugin, something like the following should solve your use case:

function waitForInputPlugin() {
  return {
    name: 'wait-for-input',
    async buildStart(options) {
      const inputSpecifiers =
        typeof options.input === 'string'
          ? [options.input]
          : Array.isArray(options.input)
          ? options.input
          : Object.keys(input);

      let lastAwaitedSpecifier = null;
      checkSpecifiers: while (true) {
        for (const specifier of inputSpecifiers) {
          if (await this.resolve(specifier) === null) {
            if (lastAwaitedSpecifier !== specifier) {
              console.log(`Waiting for input "${specifier}"...`);
              lastAwaitedSpecifier = specifier;
            }
            await new Promise(resolve => setTimeout(resolve, 500));
            continue checkSpecifiers;
          }
        }
        break;
      }
    }
  };
}

It makes use of the fact that the builtin handler for this.resolve returns null for missing files. Then it prevents starting the build process by returning a promise in buildStart that only resolves once all inputs can be found.

[Heerschop]

Thank's for the feedback, the plugin approach solve my issue very well.

To make this easier to use by others, it might be an option to make this the default behavior when using watch-mode, in this way there will be way less churn. The change will mostly be limited to the loadEntryModule method in the ModuleLoader.ts file, it could look like this than:

let resolveIdResult: ResolveIdResult;
const unresolvedFiles = new Set<string>();

while (
  (resolveIdResult = await resolveId(
    unresolvedId,
    importer,
    this.preserveSymlinks,
    this.pluginDriver,
    null
  )) === undefined && this.graph.watcher !== null
) {
  if (!unresolvedFiles.has(unresolvedId)) {
    this.graph.watcher.emit('event', {
      code: 'INPUT_WAIT',
      input: unresolvedId
    });

    unresolvedFiles.add(unresolvedId);
  }

  await new Promise<void>(resolve => {
    setTimeout(resolve, 500);
  });
}

In this approach the availability of the this.graph.watcher field will determine when to wait or not.

I'm willing to change the pull request or create a new one, if you are interested, if not i'm happy also.

[lukastaegert]

I have thought about it a little and I am still somewhat hesitant about this. Nevertheless I see that ease of use is of course valuable. On the other hand, I do not feel good about this being the default behaviour for the pure JS API as well, there may be quite a few edge case scenarios that play into this.
One thing I could get behind, though, would be to make this a feature only for CLI. The CLI already has some special plugins it injects, e.g. for stdin pipe support here

rollup/cli/run/commandPlugins.ts

Line 10 in 462bff7

inputOptions.plugins!.push(stdinPlugin());

Also it is a very well defined environment in that it always runs in Node and there are no downstream tools to consider.

The plugin could be injected somewhere here

rollup/cli/run/watch-cli.ts

Line 18 in 462bff7

export async function watch(command: any) {

What do you think? With regard to testing, you could take inspiration from the tests here https://github.com/rollup/rollup/tree/master/test/cli/samples/watch

[Heerschop]

That sound like a solid solution. I tested it and it is working, in my test the plugin is loaded in the watch-cli.ts like this:

({ options: configs, warnings } = await loadAndParseConfigFile(configFile!, command));
reloadingConfig = false;

for(const config of configs){
  if (config.plugins) {
    config.plugins.push(waitForInputPlugin());
  }
}

I can update the pull request accordingly.

For my use case it would be practical to have the option to enable the plugin for build as well.
I tried something like this rollup --config --plugin wait-for-input but then it can not find and load the plugin. It has to be made available first some how, it think.

I can still add the plugin to the rollup.config.js as well. But then it will not overload the build-in one, nevertheless that they have the same name. This will result in loading the plugin two times when using the --watch option, or is it possible to use a plugin for a particular options only?

But maybe my use case is a bit too specialised.

[lukastaegert]

For my use case it would be practical to have the option to enable the plugin for build as well

Then maybe we do it yet again slightly differently, add an option --waitForBundleInput but make it CLI only, like --env or --plugin. Then the plugin can be added together with all other plugins in addCommandPluginsToInputOptions in commandPlugins.ts, depending on this option.

[Heerschop]

That sounds good one small addition, shall we always load the plugin when in watch mode?

So the --waitForBundleInput option is not needed when using the --watch option. When doing it this way, there will be no way to disable the plugin when using the --watch option, this is not a problem is it?

[lukastaegert]

IMO if we add an option I would not set it automatically for the reason you mentioned, you cannot get rid of it. If there is a typo in the input name, it might be helpful for the build to fail even in watch-mode instead of "hanging". This will also make this very much a non-breaking change. One can still revisit making this a default for watch-mode with the next major.

[Heerschop]

I agree :-), i will update the pull request in the way you suggested.

[Heerschop]

I have updated the pull request as suggested. The test are probably not good enough but this is the best i can do.

[lukastaegert]

Looks good, I'll add some tests

[lukastaegert]

This is good from my side, unless you have any concerns I will release this tomorrow.

[Heerschop]

No concerns at all thank you and it was nice working together.