fix: regeneration should only happen for config document pattern #9051
Conversation
saihaj
requested review from
dimaMachina,
dotansimha,
kamilkisiela and
n1ru4l
🦋 Changeset detectedLatest commit: 66a48bd The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
🦋 Changeset detectedLatest commit: 254177b The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
🚀 Website PreviewThe latest changes to the website are available as preview in: https://5f19b19c.graphql-code-generator.pages.dev |
saihaj
deleted the
saihaj/9045-graphql-codegencli-320-watch-mode-regeneration-triggers-on-all-file-changes-not-just-files-specified-in-config-documents-pattern
branch
| * @parcel/watcher has no way to run watcher on specific files (https://github.com/parcel-bundler/watcher/issues/42) | ||
| * But we can use micromatch to filter out events that we don't care about | ||
| */ | ||
| if (!mm.contains(path, files)) return; |
There was a problem hiding this comment.
@saihaj I believe there's still an issue here when a negated path is used.
My use case looks like this:
documents:
- './apps/frontend/src/app/**/*.graphql'
- '!./apps/frontend/src/app/exclude-these/**/*.graphql'I'm seeing the watcher run when any file in my source tree is changed, and I'm thinking it's due to '!./apps/frontend/src/app/exclude-these/**/*.graphql' causing the mm.contains call to return true.
Quick example with mm:
const mm = require('micromatch');
// Prints false
console.log(mm.contains('anyfile.js', './some-dir/**/*.graphql'));
// Prints true
console.log(mm.contains('anyfile.js', '!./some-dir/**/*.graphql'));
There was a problem hiding this comment.
I haven't checked the code, but have noticed an excessive amount of runs in watch mode since the update myself.
There was a problem hiding this comment.
If you can open issue with repro will be easier for me to debug and fix
There was a problem hiding this comment.
I have downgraded to 3.1.0 and noticed that there is indeed a regression in newer versions causing the generator to run 5-10 times in a row (probably tracking changes in the .next folder). For now in 3.2+ I had to remove all negated paths and be more specific with included directories which helped to reduce the amount of compilations.
Unfortunantely my project is not public and I don't necesarily have sufficient time to reduce it to a MWE, but I could consider sharing the repository with you @saihaj in case you'd be willing to look into it unless somebody has a decent reproduction.
There was a problem hiding this comment.
A minor update: I found the --debug flag and confirmed that having negations causes them to be picked up causing the excess updates:
There was a problem hiding this comment.
I did a similar investigation by using --debug and modifying the source directly and can confirm the same thing that Christian is seeing. I was able to remove the negated pattern and now things work as I want them do.
Fortunately, our project doesn't need the negated pattern - I only added the negated pattern with v3.2.0 to try to fix the issue that this PR ended up fixing. I am sharing this to confirm the behavior that the other user is also seeing.
There was a problem hiding this comment.
That's a great find! Let's use this cause this PR notif got hidden in my inbox #9177
#9009 missed the fact that watch is running everywhere. Could not find a built in way with parcel but got a work around which I think is good enough for us.
closes #9045
Before
CleanShot.2023-02-23.at.13.28.35.mp4
After
CleanShot.2023-02-23.at.13.27.13.mp4