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

--watch doesn't work for multi-project #9233

Open
SeokminHong opened this issue Mar 28, 2023 · 7 comments
Open

--watch doesn't work for multi-project #9233

SeokminHong opened this issue Mar 28, 2023 · 7 comments
Labels
kind/bug Bug :-( stage/5-alpha-release-testing The pull request is merged, an alpha release is available, to be tested

Comments

@SeokminHong
Copy link

Which packages are impacted by your issue?

@graphql-codegen/cli

Describe the bug

--watch flag doesn't work for multi-project config. It throws following error:

[ProjectNotFoundError: Project 'default' not found]

Your Example Website or App

https://stackblitz.com/edit/github-kf5dtb?file=codegen.ts&view=editor

Steps to Reproduce the Bug or Issue

  1. Make a multi-project config
  2. Run graphql-codegen with --watch config
  3. Change the config file

Expected behavior

The --watch flag whould work

Screenshots or Videos

No response

Platform

  • OS: [e.g. macOS, Windows, Linux]
  • NodeJS: [e.g. 18.5.0]
  • graphql version: [e.g. 16.3.0]
  • @graphql-codegen/* version(s): [e.g. 2.6.2]

Codegen Config File

No response

Additional context

No response

@saihaj saihaj added kind/bug Bug :-( stage/1-reproduction A reproduction exists labels Mar 28, 2023
@milesrichardson
Copy link
Contributor

milesrichardson commented Apr 3, 2023

I was about to make a new issue for a similar bug, but I think it's the same underlying cause as this.

In my case the bug is that watch mode only applies to documents below the current working directory. However, it's valid config (and build mode succeeds) to have documents matched outside the current directory, e.g. (from our config):

types/console.tsx:
    schema:
      - "../gql-api/schema/unified_schema.graphql"
    documents:
      - "../console/**/*.graphql"

Changes to ../gql-api will not be picked up, because they are not below the current working directory.

I believe this is the cause of some complaints mentioned in the comments of #9009

Specifically, the bug is here:

https://github.com/dotansimha/graphql-code-generator/blob/master/packages/graphql-codegen-cli/src/utils/watcher.ts#L95-L97

      });

    watcherSubscription = await parcelWatcher.subscribe(
      process.cwd(),
      async (_, events) => {
        // it doesn't matter what has changed, need to run whole process anyway
        await Promise.all(

This only listens for changes below the current working directory. However, it's possible for glob patterns to reference changes above the current working directory.

I'm not sure what the best fix is. Ideally, we could compile all the glob patterns to pass them to @parcel/watcher. But this might introduce errors with various micromatch and glob inconsistencies. So perhaps instead, a better option would be to iterate over all patterns, find the "highest" common directory, and use that instead of process.cwd() when listening for events.

@milesrichardson
Copy link
Contributor

Here's a reproduction. However, it seems --watch doesn't actually work on Stackblitz (the command just exits immediately). You'll have to pull the repo locally to try it.

https://stackblitz.com/github/milesrichardson/repro-npm-graphql-codegen-watch-outside-cwd/?file=README.md

From repo: https://www.github.com/milesrichardson/repro-npm-graphql-codegen-watch-outside-cwd

@milesrichardson
Copy link
Contributor

I have a PR for this, incoming in next hour

@milesrichardson
Copy link
Contributor

created a separate issue for my particular use case (not clear whether it fixes this one): #9266

opened PR with fix: #9267

@saihaj
Copy link
Collaborator

saihaj commented Apr 4, 2023

@SeokminHong can you test @graphql-codegen/cli@3.3.1-rc-20230404030530-47bcfb095

@SeokminHong
Copy link
Author

SeokminHong commented Apr 5, 2023

I tested it for my project and it works! Thank you!

@milesrichardson
Copy link
Contributor

milesrichardson commented Apr 5, 2023

@SeokminHong Nice, glad to hear it's fixed for you 😄 Note that you'll probably notice watch mode is now pretty aggressive, since it's using the "highest common directory," and if that's your repo root, it will rebuild on changes within .next, or even changes to .git/index.log when you run git status. This was actually the case before, too, and would be affecting anyone using it at the root of their project, so it's a separate issue, but if you have a mono-repo like I do, it might now be particularly noticeable after this change.

This is tracked in #9270 and should be fixed in #9275, so keep an eye out for that to be merged, and make sure to update once that release is available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Bug :-( stage/5-alpha-release-testing The pull request is merged, an alpha release is available, to be tested
Projects
None yet
Development

No branches or pull requests

3 participants