The default shouldPreload logic seems incorrect

[philipwalton]

The default shouldPreload function in this plugin will preload everything that is a dynamic import or has exports (without being a facade chunk). I'm not sure why that criteria was chosen.

Normally you want to preload everything in your initial static module graph, i.e. all the things that the browser would naturally load if given your primary module entry point. The idea is you want to tell the browser ahead of time what's going to be in the graph (so it can make the requests in parallel, without having to discover them).

In some cases you may also want to preload certain modules in the graph of a dynamic entry point (e.g. a router that dynamically loads all routes), but most of the time the reason they're in a dynamic entry point is they're conditionally loaded, so preloading them would defeat the purpose of conditionally loading them.

I think the default logic to determine the list of modules to preload should be: Every chunk that's in the module graph of all static entry points.

Then, optionally, a developer may choose to specify dynamic entry chunks whose graph they also want to preload.

Lastly, I could see a use case for never preloading certain modules, so it probably makes sense to have a filter.

How does that sound? If you're interested in these changes I'd be happy to submit a PR. To give you an idea of what I'm thinking, here's how I'm currently getting the preload graphs for my entry modules (note: you do have to builds these graphs yourself, as this information isn't exposed by anything in ChunkInfo):

{
  generateBundle(options, bundle) {
    // A mapping of entry chunk names to all dependencies in their
    // static graphs
    const entryChunksMap = {};

    // Loop through all the chunks to detect entries.
    for (const [fileName, chunkInfo] of Object.entries(bundle)) {
      // This logic gets the graphs for both static and dynamic entries
      // but you could imagine making it possible for a user to custom
      // the conditional in this if-statement.
      if (chunkInfo.isEntry || chunkInfo.isDynamicEntry) {
        // A set of chunks in this entries graph.
        const entryGraph = new Set([fileName]);

        // A set of checked chunks to avoid circular dependencies.
        const seen = new Set();

        const imports = chunkInfo.imports.slice();
        let importFileName;
        while (importFileName = imports.shift()) {
          const importChunkInfo = bundle[importFileName];

          entryGraph.add(importChunkInfo.fileName);

          // Add all inner chunk imports to the queue, checking for
          // circular dependencies since chunks can import each other.
          for (const i of importChunkInfo.imports) {
            if (!seen.has(i)) {
              seen.add(i);
              imports.push(i);
            }
          }
        }
        entryChunksMap[chunkInfo.name] = [...entryGraph];
      }
    }

    // Expose this somehow so the developer can create `modulepreload``
    // tags base on these graphs.
    console.log(entryChunksMap);
  }
}