Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Watcher backends: use Flow-typed object payloads #1411

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

robhogan
Copy link
Contributor

Summary:
Ensure 'add' and 'change' events fired by watcher backends always have associated metadata (and that 'delete' events never do), by typing change event payloads as objects with the event: 'change' | 'add' | 'delete' as a discriminator.

(This is in preparation for refactoring out the distinction between 'change' and 'add' from backends, which means they don't need to track files and can do much less IO)

Changelog: Internal

Differential Revision: D67287046

…cy (#1407)

Summary:
We currently use the [`anymatch`](https://www.npmjs.com/package/anymatch) dependency to evaluate ignore patterns in watcher backends. `anymatch` handles various input types including globs and arrays of globs/regexps

However, since D67285745, it's become clear that the `ignored` param is only ever a `?RegExp`, so we hardly use any of `anymatch` and can simply `test()` paths, and remove the dependency.

One subtlety to note that this change makes explicit, is that `anymatch` converts all input paths to posix separators before testing against the input `RegExp` (see block comment). We do the same in this diff so that the change is behaviour-preserving - however, this isn't a convention in Metro and should be revisited (maybe in a breaking change).

Note that we don't need to handle other kinds of path normalisation (like `anymatch` does with [`normalize-path`](https://www.npmjs.com/package/normalize-path)) because watcher backends must already emit normal paths, with no trailing separators. 

Changelog: Internal


Reviewed By: hoxyq

Differential Revision: D67259981
Summary:
Currently watcher backends, which extend `EventEmitter`, emit two events for any file change:
 - One with event name `change`/`add`/`delete`
 - One with event name `all`, and `change`/`add`/`delete` in the payload.

We only ever listen to `all`. The former type is neither exercised nor tested. This removes the unused events.

Changelog: Internal

Reviewed By: hoxyq

Differential Revision: D67393484
Summary:
Ensure `'add'` and `'change'` events fired by watcher backends always have associated `metadata` (and that `'delete'` events never do), by typing change event payloads as objects with the `event: 'change' | 'add' | 'delete'` as a discriminator.

(This is in preparation for refactoring out the distinction between `'change'` and `'add'` from backends, which means they don't need to track files and can do much less IO)

Changelog: Internal

Differential Revision: D67287046
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 21, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67287046

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants