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

fix(gatsby): work around webpack watching issue #30194

Closed
wants to merge 9 commits into from

Conversation

vladar
Copy link
Contributor

@vladar vladar commented Mar 11, 2021

Description

This PR aims to fix "double save" issues in Gatsby (when some saves are being lost). The issue is caused by resume and suspend flow of webpack. There are (at least) two issues in webpack itself with this flow that make it hard to make use of suspend and resume at random points in time:

webpack/webpack#12882
webpack/webpack#12898

If those issues are fixed, then this PR won't be needed. Otherwise, we will have to properly coordinate calls of resume and suspend to make sure they are invoked at specific moments (and when webpack is in a specific state).

I've published a canary gatsby@3.1.0-alpha-hmr-fix.16+831f9a9d64 with this fix for anyone who wants to try it.

Related Issues

Fixes #27609

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Mar 11, 2021
@vladar vladar added topic: webpack/babel Webpack or babel and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Mar 11, 2021
Comment on lines -10 to +24
callback({ type: `SOURCE_FILE_CHANGED`, file })

// Webpack fires `invalid` event as soon as any file changes
// but it doesn't start recompiling at this point. Instead, it aggregates changes with debounce
// and recompile on a tail of debounce.
// For example, you may save a file multiple times quickly but webpack will only recompile once.
// Unfortunately webpack doesn't expose any event for aggregated changes.
// But we actually need it. If we start recompiling immediately on `invalid` we can miss some changes.
// Hack below is a workaround for this missing webpack event
// @ts-ignore
const watcher = compiler.watchFileSystem.watcher // Watchpack instance
watcher.once(`aggregated`, () => {
callback({ type: `SOURCE_FILE_CHANGED`, file })
})

// TODO: fallback to timeout?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from my own investigations - sometimes webpack/watchpack can emit aggregated without emitting invalid before - should this maybe be hoisted and just listening to watcher.on('aggregated') and just avoid using hooks.invalid?

Copy link
Contributor Author

@vladar vladar Mar 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is tricky because each compiliation has a new compiler.watchFileSystem.watcher instance (see here). And we want to listen to current active instance. But also - if it doesn't emit invalid - how do we even react to this now? Does it mean we don't emit SOURCE_FILE_CHANGED at all? Or we do but in query-watcher?

@vladar
Copy link
Contributor Author

vladar commented Jun 25, 2021

The correct fix for this shipped in webpack: webpack/webpack#13399

So closing.

@vladar vladar closed this Jun 25, 2021
@LekoArts LekoArts deleted the vladar/fix-webpack-watching branch June 25, 2021 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: webpack/babel Webpack or babel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gatsby doesn't always refresh changes
2 participants