-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
refactor(gatsby): use virtual modules for (a)sync-requires
and match-paths.json
#25057
Conversation
instances.forEach(instance => { | ||
instance.writeModule(adjustedFilePath, fileContents) | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really cool. i would love to have a header comment explaining what this module can do so in the future when a dev is reading this can know in what cases they might want to reuse this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will add some description/context, good point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very cool! Approving cause the code looks fine, but just would love more comments explaining how this system works and why it's beneficial
oh and tests are failing i guess.. haha
4c1696e
to
ca13d28
Compare
388f7fa
to
d8a49e3
Compare
d8a49e3
to
c3c0302
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and worked like a charm! 💪
9e32e72
to
1425b55
Compare
c4b0297
to
56d2938
Compare
Your pull request can be previewed in Gatsby Cloud: https://build-32835a8f-a9e9-43fb-817d-78742faf3c08.staging-previews.gtsb.io |
Description
This avoids adding
fs
involved when we dynamically generate(a)sync-requires
andmatch-paths.json
files.Why using
fs
is problematic?It adds unnecessary async code to the pipeline, writing is fine (in fact I don't remove writing in this PR - see comment in
requires-writer.ts
for reaosns). It's about when we (or rather webpack watching) react to fs changes events - webpack watcher have some debounced/aggregation logic (200ms afaik) + any system level delays (from writing to fs, to receiving FS change event in watchpack/chokidar).This PR on its own is not solving lot of problems, but it makes it easier for future PRs that try to coordinate our services to avoid pitfalls of some debounced state transitions
[CH9862]