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

feat: use @parcel/watcher #9009

Merged
merged 4 commits into from
Feb 21, 2023
Merged

feat: use @parcel/watcher #9009

merged 4 commits into from
Feb 21, 2023

Conversation

saihaj
Copy link
Collaborator

@saihaj saihaj commented Feb 18, 2023

As the title says use @parcel/watcher over chokidar. It is way performant and watch mode feels fast! Based on platform it uses has the following watcher backends, listed in priority order:

  1. FSEvents on macOS
  2. Watchman if installed
  3. inotify on Linux
  4. [ReadDirectoryChangesW](https://msdn.microsoft.com/en us/library/windows/desktop/aa365465%28v%3Dvs.85%29.aspx) on Windows

One thing to note with this PR is we can no longer support polling config which was a feature in chokidar so things like #5580 will not really be possible. There is an open issue related to this parcel-bundler/watcher#99

closes #8963

@changeset-bot
Copy link

changeset-bot bot commented Feb 18, 2023

🦋 Changeset detected

Latest commit: e01f01d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@graphql-codegen/cli Minor
@graphql-cli/codegen Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Feb 18, 2023

🚀 Snapshot Release (alpha)

The latest changes of this PR are available as alpha on npm (based on the declared changesets):

Package Version Info
@graphql-cli/codegen 3.0.1-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/cli 3.1.0-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/core 3.0.1-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/add 4.0.1-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/fragment-matcher 4.0.1-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/introspection 3.0.1-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/schema-ast 3.0.1-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/visitor-plugin-common 3.0.1-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/typescript-document-nodes 3.0.1-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/gql-tag-operations 2.0.1-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/typescript-operations 3.0.1-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/typescript-resolvers 3.1.0-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/typed-document-node 3.0.1-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/typescript 3.0.1-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/client-preset 2.1.0-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/gql-tag-operations-preset 2.1.0-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/graphql-modules-preset 3.0.1-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/testing 2.0.1-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎
@graphql-codegen/plugin-helpers 4.1.0-alpha-20230220220141-e01f01d51 npm ↗︎ unpkg ↗︎

@github-actions
Copy link
Contributor

github-actions bot commented Feb 18, 2023

🚀 Website Preview

The latest changes to the website are available as preview in: https://e2b4dc7a.graphql-code-generator.pages.dev

@github-actions
Copy link
Contributor

diff --git a/website/algolia-lockfile.json b/website/algolia-lockfile.json%0Aindex 5b7e49e7d..6b6d6882a 100644%0A--- a/website/algolia-lockfile.json%0A+++ b/website/algolia-lockfile.json%0A@@ -527,7 +527,7 @@%0A         "anchor": "whats-next"%0A       }%0A     ],%0A-    "content": "241d25c61530cb506ca80af52932b631",%0A+    "content": "4e75b012b94c2a9880279bf0f63401ab",%0A     "url": "https://www.the-guild.dev/graphql/codegen/docs/getting-started/development-workflow",%0A     "domain": "https://www.the-guild.dev/graphql/codegen/",%0A     "hierarchy": [

@saihaj saihaj marked this pull request as ready for review February 20, 2023 22:03
@n1ru4l
Copy link
Collaborator

n1ru4l commented Feb 21, 2023

@saihaj Can this solve #8963?

@saihaj
Copy link
Collaborator Author

saihaj commented Feb 21, 2023

@saihaj Can this solve #8963?

Looks like so

CleanShot.2023-02-21.at.11.13.21.mp4

@saihaj saihaj merged commit 288ed09 into master Feb 21, 2023
@saihaj saihaj deleted the saihaj/parcel/watcher branch February 21, 2023 17:33
@saihaj
Copy link
Collaborator Author

saihaj commented Feb 21, 2023

/theguild newsletter

@lauraseidler
Copy link

Looks like this introduces a dependency on node-gyp, and with that on Python, which at least for us, is a breaking change - we are using node js alpine docker images, which do not have python installed by default, so this requires a change to the build process.

@ardatan
Copy link
Collaborator

ardatan commented Feb 24, 2023

@lauraseidler I am not if it can be considered as a breaking change. node-gyp is the native part of Node.js ecosystem, and GraphQL Codegen's CLI was already Node.js specific package. Python is one of prerequisites of Node.js as far as I know.

@szilarddoro
Copy link

I can confirm that this is a breaking change for alpine images. We are 99% sure that our pipeline started failing because of @parcel/watcher:

Our builder step didn't have python installed, so we had to change our Dockerfile.

@mogelbrod
Copy link

mogelbrod commented Mar 3, 2023

Looks like the switch to @parcel/watcher may have also resulted in negated glob patterns no longer working as expected. Using the following documents list I can see that this line doesn't bail as expected for events that should be ignored (such as create .git/.watchman-cookie-name-869-12641), but happily continues:

documents:
  - "app/**/!(*.d).{graphql,ts,tsx,js,jsx}"
  - "!app/graphql/**"

This results in codegen running multiple times each time a file is changed.

@milesrichardson
Copy link
Contributor

milesrichardson commented Apr 3, 2023

Watch mode broke for us (it simply doesn't pickup changes) when migrating from 1.21.3 to 3.2.2. Granted, that's a large change, so I've been trying to track down what broke it.

In our case, the breakage is that changes are not picked up.

EDIT:

I located the source of the bug. The issue is that process.cwd() is passed as the argument to parcelWatcher.subscribe, but our config includes paths that are outside of ("above") the current directory, which means they're not included in the watch patterns. I believe this issue is already tracked (albeit with a slightly different method of reproduction) in #9233

milesrichardson added a commit to milesforks/graphql-code-generator that referenced this pull request Apr 3, 2023
Fixes dotansimha#9266

Prior to dotansimha#9009, which moved from Chokidar to `@parcel/watcher`, the behavior
was to watch for all relevant files. However, since switching to
`@parcel/watcher`, the new behavior has been to watch all files below
`process.cwd()`, and then filter the change events for only the relevant files.
This approach works fine, except that it's possible for a valid config to
reference paths outside of the current working directory (e.g. `documents:
"../some-other/*.graphql`), and these paths are included in build mode, but
were not being included in watch mode because they're outside of
`process.cwd()`.

This commit adds logic, after parsing all relevant file paths, to find the
"longest common directory prefix" of all those file paths, i.e. the "highest"
(closest to `/`) directory that contains all the relevant file paths. Then,
when subscribing to the Parcel watcher, this directory is used instead of
`process.cwd()`. For example, the longest common directory of the paths
`/foo/bar/*.graphql` and `/foo/fizz/*.graphql` would be `/foo`.

Note that the filtering behavior is left unchanged, and this only affects the
root path given to the Parcel watcher. When an event is received, the filtering
can still filter out irrelevant paths, including those filtered by
`config.watchPattern` if it's defined.
milesrichardson added a commit to milesforks/graphql-code-generator that referenced this pull request Apr 3, 2023
Prior to dotansimha#9009, which moved from Chokidar to `@parcel/watcher`, the behavior
was to watch for all relevant files. However, since switching to
`@parcel/watcher`, the new behavior has been to watch all files below
`process.cwd()`, and then filter the change events for only the relevant files.
This approach works fine, except that it's possible for a valid config to
reference paths outside of the current working directory (e.g. `documents:
"../some-other/*.graphql`), and these paths are included in build mode, but
were not being included in watch mode because they're outside of
`process.cwd()`.

This commit adds logic, after parsing all relevant file paths, to find the
"longest common directory prefix" of all those file paths, i.e. the "highest"
(closest to `/`) directory that contains all the relevant file paths. Then,
when subscribing to the Parcel watcher, this directory is used instead of
`process.cwd()`. For example, the longest common directory of the paths
`/foo/bar/*.graphql` and `/foo/fizz/*.graphql` would be `/foo`.

Note that the filtering behavior is left unchanged, and this only affects the
root path given to the Parcel watcher. When an event is received, the filtering
can still filter out irrelevant paths, including those filtered by
`config.watchPattern` if it's defined.
milesrichardson added a commit to milesforks/graphql-code-generator that referenced this pull request Apr 3, 2023
Fixes dotansimha#9266

Prior to dotansimha#9009, which moved from Chokidar to `@parcel/watcher`, the behavior
was to watch for all relevant files. However, since switching to
`@parcel/watcher`, the new behavior has been to watch all files below
`process.cwd()`, and then filter the change events for only the relevant files.
This approach works fine, except that it's possible for a valid config to
reference paths outside of the current working directory (e.g. `documents:
"../some-other/*.graphql`), and these paths are included in build mode, but
were not being included in watch mode because they're outside of
`process.cwd()`.

This commit adds logic, after parsing all relevant file paths, to find the
"longest common directory prefix" of all those file paths, i.e. the "highest"
(closest to `/`) directory that contains all the relevant file paths. Then,
when subscribing to the Parcel watcher, this directory is used instead of
`process.cwd()`. For example, the longest common directory of the paths
`/foo/bar/*.graphql` and `/foo/fizz/*.graphql` would be `/foo`.

Note that the filtering behavior is left unchanged, and this only affects the
root path given to the Parcel watcher. When an event is received, the filtering
can still filter out irrelevant paths, including those filtered by
`config.watchPattern` if it's defined.
milesrichardson added a commit to milesforks/graphql-code-generator that referenced this pull request Apr 3, 2023
Fixes dotansimha#9266

Prior to dotansimha#9009, which moved from Chokidar to `@parcel/watcher`, the behavior
was to watch for all relevant files. However, since switching to
`@parcel/watcher`, the new behavior has been to watch all files below
`process.cwd()`, and then filter the change events for only the relevant files.
This approach works fine, except that it's possible for a valid config to
reference paths outside of the current working directory (e.g. `documents:
"../some-other/*.graphql`), and these paths are included in build mode, but
were not being included in watch mode because they're outside of
`process.cwd()`.

This commit adds logic, after parsing all relevant file paths, to find the
"longest common directory prefix" of all those file paths, i.e. the "highest"
(closest to `/`) directory that contains all the relevant file paths. Then,
when subscribing to the Parcel watcher, this directory is used instead of
`process.cwd()`. For example, the longest common directory of the paths
`/foo/bar/*.graphql` and `/foo/fizz/*.graphql` would be `/foo`.

Note that the filtering behavior is left unchanged, and this only affects the
root path given to the Parcel watcher. When an event is received, the filtering
can still filter out irrelevant paths, including those filtered by
`config.watchPattern` if it's defined.
milesrichardson added a commit to milesforks/graphql-code-generator that referenced this pull request Apr 3, 2023
Prior to dotansimha#9009, which moved from Chokidar to `@parcel/watcher`, the behavior
was to watch for all relevant files. However, since switching to
`@parcel/watcher`, the new behavior has been to watch all files below
`process.cwd()`, and then filter the change events for only the relevant files.
This approach works fine, except that it's possible for a valid config to
reference paths outside of the current working directory (e.g. `documents:
"../some-other/*.graphql`), and these paths are included in build mode, but
were not being included in watch mode because they're outside of
`process.cwd()`.

This commit adds logic, after parsing all relevant file paths, to find the
"longest common directory prefix" of all those file paths, i.e. the "highest"
(closest to `/`) directory that contains all the relevant file paths. Then,
when subscribing to the Parcel watcher, this directory is used instead of
`process.cwd()`. For example, the longest common directory of the paths
`/foo/bar/*.graphql` and `/foo/fizz/*.graphql` would be `/foo`.

Note that the filtering behavior is left unchanged, and this only affects the
root path given to the Parcel watcher. When an event is received, the filtering
can still filter out irrelevant paths, including those filtered by
`config.watchPattern` if it's defined.
saihaj pushed a commit that referenced this pull request Apr 4, 2023
…ry prefix" of relevant files, rather than only files below `process.cwd()`, while keeping event filtering intact (#9267)

* fix: Listen for file changes in watch mode in longest common directory

Prior to #9009, which moved from Chokidar to `@parcel/watcher`, the behavior
was to watch for all relevant files. However, since switching to
`@parcel/watcher`, the new behavior has been to watch all files below
`process.cwd()`, and then filter the change events for only the relevant files.
This approach works fine, except that it's possible for a valid config to
reference paths outside of the current working directory (e.g. `documents:
"../some-other/*.graphql`), and these paths are included in build mode, but
were not being included in watch mode because they're outside of
`process.cwd()`.

This commit adds logic, after parsing all relevant file paths, to find the
"longest common directory prefix" of all those file paths, i.e. the "highest"
(closest to `/`) directory that contains all the relevant file paths. Then,
when subscribing to the Parcel watcher, this directory is used instead of
`process.cwd()`. For example, the longest common directory of the paths
`/foo/bar/*.graphql` and `/foo/fizz/*.graphql` would be `/foo`.

Note that the filtering behavior is left unchanged, and this only affects the
root path given to the Parcel watcher. When an event is received, the filtering
can still filter out irrelevant paths, including those filtered by
`config.watchPattern` if it's defined.

* chore: Add `yarn watch:examples` and `dev-test-outer-dir` for watch mode testing

This adds a directory "outside" of `dev-test`, and also adds a new script
command `yarn watch:examples` which is the "watch mode" equivalent of
`yarn generate:examples`. It adds a dependency on a `githunt` document in
`dev-test-outer-dir`, and if all is working as expected, you should be able to
edit this file while in watch mode to trigger a rebuild, and it should also be
included in build mode. That is, if you run `yarn generate:examples`, or
`yarn watch:examples`, there should be no changes to the Git repo.
@xli1000
Copy link

xli1000 commented Apr 5, 2023

We also encountered problems building @parcel/watcher when upgrading. node-gyp dependencies seem to be much more troublesome/inconvenient than regular JS dependencies because there are often differences in the environment that cause the build to fail or there are network issues with node-gyp requesting certain files when building behind a corporate proxy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

watch mode should pick up codegen config changes
9 participants