fix(gatsby-dev-cli): increase debounce threshold in CI environments #8387
Conversation
|
Huh weird. Who wrote that code 😅 Can't we do a |
|
@KyleAMathews I was refactoring it to go that way. I'll continue that here 😄 |
packages/gatsby-dev-cli/src/watch.js
Outdated
| @@ -8,21 +8,23 @@ let numCopied = 0 | |||
| const debouncedQuit = _.debounce(() => { | |||
There was a problem hiding this comment.
Do we still need to debounce?
There was a problem hiding this comment.
I think so. I think the watcher gets triggered pretty often, and so the debounce is necessary so that we don't quit too soon.
That said, that's more of a guess than a firm guarantee 🤷♂️
There was a problem hiding this comment.
Oh I see... yeah, on scan-once we should only listen to the ready event then as that's only called once Chokidar has scanned all files https://github.com/paulmillr/chokidar
There was a problem hiding this comment.
Neat :) I'll fix this in a sec. Deep in caching stuff 😅
| @@ -5,10 +5,10 @@ const syspath = require(`path`) | |||
|
|
|||
| let numCopied = 0 | |||
|
|
|||
| const debouncedQuit = _.debounce(() => { | |||
| const quit = () => { | |||
There was a problem hiding this comment.
No need for debounce anymore. We know it's done when .on('ready') fires.
packages/gatsby-dev-cli/src/watch.js
Outdated
| ].concat( | ||
| packages.map(p => new RegExp(`${p}[\\/\\\\]src[\\/\\\\]`, `i`)) | ||
| ) | ||
| const watchers = packages.map(p => syspath.join(root, `/packages/`, p)) |
There was a problem hiding this comment.
Set up an array of packages to watch, rather than iterating with a .forEach (which is kinda janky, and .on('ready') will fire for every package.
packages/gatsby-dev-cli/src/watch.js
Outdated
| }) | ||
| .on(`all`, (event, path) => { | ||
| if (event === `change` || event === `add`) { | ||
| const name = syspath.basename(syspath.dirname(path.split(`packages/`).pop())) |
There was a problem hiding this comment.
A little janky, but we need to infer the name from the full watched file path.
|
Tests are passing--although I think that was more luck than anything else. The e2e tests don't use this version of gatsby-dev-cli, they use the latest available. |
DSchau
changed the title
* 'master' of github.com:gatsbyjs/gatsby: chore(release): Publish fix: add missing html and body attributes from ssr apis to develop mode (gatsbyjs#8594) chore(www): bump gatsby version (gatsbyjs#8614) chore: upgrade Gatsby Version in Gatsbygram Readme (gatsbyjs#8604) fix(gatsby-dev-cli): wait for files to be copied before exiting (gatsbyjs#8387) fix: add missing GatsbyImageProps to TS defs (gatsbyjs#8606) chore: forgot frontmatter date change (gatsbyjs#8601) Add Hasura GraphQL engine example under third party (gatsbyjs#8584) blog: update incorrect post date (gatsbyjs#8600) Add gatsby-mdx-starter to the list of starters (gatsbyjs#8547) Draft graphiql doc (gatsbyjs#8599) Added redirect (gatsbyjs#8528) feat: provide fragments without gatsby-image (gatsbyjs#8459) updated CODEOWNERS (gatsbyjs#8591) Updates to sidebar (gatsbyjs#8539)
I think this might fix #8381.
I think the process was exiting too quickly in the CI environment, therefore the task was stopped before all the files could copy over.
Also note I tested the issue (not the fix) by
sshing to the CircleCI container, and running the following commandsit would occasionally copy the expected amount of files, and then sometimes 0, leading me to believe this fix may be the trick 🤞