Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Trigger rebuilds in watch mode while respecting rules of precedence a…
…nd negation (#9275) * chore: Add two new `dev-test/star-wars` queries which can be excluded by micromatch patterns This commit adds `ExcludeQueryAlpha` and `ExcludeQueryBeta`, and then checks them in along with the result of running `yarn generate:examples`, since this should be the baseline of the unmodified codegen. A separate commit will add some new outputs to the `star-wars` example that alternately exclude and include these files, in order to test pattern matching works with both generating and watching the files. * chore: Add `dev-test/types.excludeQuery{Alpha,Beta}.ts` for exclusion baseline testing This commit adds a baseline for testing pattern matching with contravariant negations between two config stanzas. That is, the `types.excludeQueryAlpha.ts` target produces an output that includes `ExcludeQueryBeta` but not `ExcludeQueryAlpha`, and the `types.excludeQueryBeta.ts` target produces an output that includes `ExcludeQueryAlpha` but not `ExcludeQueryBeta`. Manual verification shows that the output files correctly exclude these types when running generation, and so they can be checked in as a baseline for later tests to detect a regression if that changes. Otherwise, they'll be used primarily for testing `yarn watch:examples`, which we expect will adhere to the same exclusion rules during _generation_, but which we need to make ure also adheres to those rules during watch mode, while filtering file change events to trigger (or not trigger) rebuilds. * chore: Add `nothing-should-use-this-query.graphql` Add a file in `dev-test-outer-dir` which is excluded by the pattern that's part of a pattern set otherwise matching `*.graphql` files in that directory. This way, we can check that touching this file does not ever trigger a rebuild during watch mode (and also note that this commits shows it's not included in any output during generation). In fact, the bug we're fixing in watch mode is so sensitive that we can simply run `git status` to trigger a change to the Git `index.lock`, but this way at least we have a real `*.graphql` file we can touch and watch for avoidance of rebuild triggers. * fix(watch): Remove double-concatenated `fullPath`, since `path` is already absolute Parcel Watcher always provides `path` as an absolute path (see the readme for the package), and we were incorrectly concatenating it with absolute path `watchDirectory` (and prior to #9270, with `process.cwd()`, which is also an absolute path). This removes the concatenation, which should fix a potential bug when comparing to `config.configFilePath`. However, it also means that since #9050, we've been passing an absolute path to the `onWatchTriggered` hook, which may or may not be expected by downstream user code. This commit leaves that unchanged, and we still pass it an absolute path. * fix(watch): Respect negation and precedence when triggering rebuilds Implement the algorithm for deciding when to trigger a rebuild during watch mode, in such a way that precedence (both in terms of global config preceding per-target config, and in terms of watch patterns preceding document/schema patterns) is respected, and negations are evaluated within their local groups. This algorithm is described and diagrammed here: #9270 (comment) * chore(tests): Return `stopWatching` callback from `createWatcher` Make `createWatcher` more testable, by returning a `stopWatching` callback that can be called to stop the otherwise infinitely running watcher. Implement it by adding an abort signal handler to the existing process.exit and SIGINT handlers, to call `shutdown()` in the same way. * Add mocking infrastructure for `createWatcher` and `@parcel/watcher` Create mocking infrastructure for the watcher, including mocking `@parcel/watcher`, so that tests can dispatch file change events and spy on the subscription callback. * fix(watcher): Improve watcher shutdown precision, add more tests This commit implements graceful shutdown for the watcher, including when cancelled via an AbortSignal. It also includes a polyfill for AbortController, which is necessary on Node v14 (at least without passing any experimental flags). Shutdown events happen in logical order, and as long as the caller does `await stopRunning()`, it's guaranteed the promise will only resolve after the watch server has terminated completely. * chore(tests): Add helpers for asserting whether multiple files triggered rebuild Add a helper function that takes a mock watcher, a list of files that should trigger rebuild, and a list of files that should not trigger rebuild, and then automatically tests all of them. Call the `expect()` functions with an assertion helper that ensures the stack trace shows the relevant functon call, and also adds a formatted prelude clearly showing which path was expected to trigger or not trigger a rebuild, and what happened instead. * fix(watcher): Convert ParcelWatcher ignore paths to be relative to watchDirectory ParcelWatcher expects the `ParcelWatcher.Options["ignore"]` array to include either absolute paths, or relative paths that are relative to the `watchDirectory`. This commit fixes a bug where the paths of ignored files (i.e., output directories from stanzas with presetConfig) were relative to `process.cwd()`, which may differ from `watchDirectory` * chore(tests): Add assertions for ParcelWatcher.Options.ignore, and refactor Add mocks, assertion helpers, and error formatters for testing the value of paths and globs passed to the `options["ignore"]` argument of `parcelWatcher.subscribe()`. Note that we can't properly mock the behavior of this, because ParcelWatcher implements its ignore logic in C++, so the best we can do is make assertions on the argument it receives. This commit also refactors all of the watch mode testing infrastructure to be in a `tests/watch-helpers` directory. * chore(tests): Fix some typos, namely preferring "relative from" vs. "relative to" The "relative from" nomenclature matches how NodeJS docs refer to the concept of one path being relative from another, and in the case of changes to comments, also matches the actual variable names which were already using `from` instead of `to` * fix(watcher): Ignore negated patterns when picking watchDirectory When picking `watchDirectory` by finding the "longest common prefix directory" of all paths and glob patterns, only include affirmative (non-negated) patterns. Otherwise, a rule like `!**/never.graphql` will cause the `watchDirectory` to always be `process.cwd()`, because the result of `mm.scan("!**/never.graphql").base` is `""`. * fix(watcher): Fix logic error in negation precedence, and add more test cases Remove `pathMatches` shortcut in favor of explicitly checking for negation after finding an affirmative match, because it turns out this doesn't work. Now the code exactly matches the flow chart. The reason for this change is that micromatch does not give negation priority when there is an exact affirmative match, but we expect it to (because in our case, negations _always_ take precedence). The micromatch behavior makes sense when you're considering one set of patterns, because why would you both include and exclude an exact path? But we have the additional complexity of mixing global and local pattern sets, so we can naturally arrive at a situation where a set includes both an exact negated match, and an exact affirmative match. In this case we still want to negate it. Without this fix, the logic was not working as expected, because a global negation was being ignored when there was a local inclusion. * chore(watcher): Add isolated test for each case in addition to combined one For each individual assertion we were making in the big combined config, isolate it into a single test that captures the important aspect of it, and also add some more specificity to the logic of some of the assertions, which is much easier to see when isolated within a single test. This should make it easier to identify the source of any future regressions. Still, keep the existing test with the big combined config, because it can't hurt. But change "foo" to "fuzz" in that test, just to create different strings in case someone wants to grep for a path in a failing individual test. * fix(watcher): Import `AbortController` from `@whatwg-node/fetch` There is no need for a poly/pony-fill. * Revert "fix(watcher): Import `AbortController` from `@whatwg-node/fetch`" This reverts commit 7a374c2. There is some issue with the implementation of `AbortController` in `@whatwg-node/fetch` which causes all the tests to timeout (presumably because the `AbortSignal` is either not being emitted or received).
- Loading branch information
1 parent
c56724e
commit 2a5da58