feat(server): invalidate module when watch files change

[mattcompiles]

Description

As specified in the rollup addWatchFile docs.

Using this.addWatchFile from within the transform hook will make sure the transform hook is also reevaluated for this module if the watched file changes.

This change will invalidate the current module if any changes are detected from the files passed to this.addWatchFile during the transform hook.

Resolves #3216

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[Shinigami92]

I restarted the GH Actions, but the windows container failed again!
So looks like an issue with windows 👀

[patak-dev]

I think this solution is a good way to implement this feature. But, at least to my interpretation of the module graph, this is changing the meaning of moduleGraph.fileToModulesMap

  // a single file may corresponds to multiple modules with different queries
  fileToModulesMap = new Map<string, Set<ModuleNode>>()

IIUC, the fileMappedModules sets currently means the "list all the modules that are generated from a file" (so in a .vue file, you'll get a CSS module and a JS module). After this PR, the fileMappedModules will end up meaning the "list of all modules that needs to be invalidated after a file change"

In my opinion, this is ok. The alternative would be to add an extra watchFileMappedModules and a new getModulesToInvalidateForFile function to the module graph that would return both module node sets for the file. I don't know how important is to keep the current meaning for fileMappedModules to be able to make this call. I'll bring this PR to the team next time we discuss if it is not yet reviewed/merged by others.

[mattcompiles]

@patak-js Thanks for the review. Understand your comments. I was just trying to add the feature while creating the least amount of new concepts. Happy for any feedback on approach or to rename fileMappedModules to something more appropriate.

[patak-dev]

Lets see what others think, or we'll discuss it in a few days with the team 👍🏼

[patak-dev]

@mattcompiles what do you think if replace addWatchModuleToFile with

  ensureModulesByFile(file: string): void {
    file = normalizePath(file)
    let fileMappedModules = this.fileToModulesMap.get(file)
    if (!fileMappedModules) {
      fileMappedModules = new Set()
      this.fileToModulesMap.set(file, fileMappedModules)
    }
    return fileMappedModules
  }

So we can then use it in transformRequest.ts.

      transformResult.watchFiles.forEach((file) => {
        moduleGraph.ensureModulesByFile(file).add(mod)
      })

If we end up going this way, I think it is better to avoid the ( mod -> mod.id -> mod ) currently present in the PR. ensureModulesByFile could also be used in another PR to refactor and simplify a few function implementations in the moduleGraph.

Something I forgot yesterday, last time that we discussed #3216 we saw that there was a lot of support (a lot of +1 reactions) but there wasn't much in concrete examples of where this feature would help. If you have a use case for this feature, it would be great if you could add a comment in the issue describing it so we leave it as reference.

[mattcompiles]

Happy for any suggested strategies by the core team. I'll wait for a final opinion before updating the code though if that's ok.

Added my use-case in the issue.

[mattcompiles]

Closing in favour of db4ba56

[patak-dev]

I see that Evan added the watched files as extra imports at the end 👍🏼

@mattcompiles, the tests in this PR are still valuable. Would you like to reopen the PR and repurpose it for them?
I was thinking that we could rename the test playground from transform-plugin to plugin-api so we can later add more tests related to rollup compat and the inner working of the plugin API.