Skip to content

Conversation

86
Copy link
Contributor

@86 86 commented Nov 22, 2023

In this PR, I have added a check for the nullability of finalWatchFileSystem.watcher

Background

This library is used by the development server that starts up during Gatsby development.

Gatsby features a function named DEV_SSR, which makes it easier to detect SSR errors during development. However, when this feature is enabled in our environment and an actual SSR error occurs, the development server itself shuts down due to the following error in this library:

Cannot read properties of null (reading 'fileWatchers')

This prevents us from verifying the content of the SSR error, defeating the purpose of the DEV_SSR feature. While I have not fully investigated why finalWatchFileSystem.watcher becomes null, I believe that the modification proposed in this PR will not have any adverse effects. I would appreciate it if you could consider merging this change.

@larixer larixer merged commit e33dca4 into sysgears:master Nov 22, 2023
@larixer
Copy link
Member

larixer commented Nov 22, 2023

No problem, lets merge it. Published in version 0.6.1

@86 86 deleted the 86/avoid-filewatcher-null branch November 23, 2023 23:09
serhalp added a commit to gatsbyjs/gatsby that referenced this pull request Aug 14, 2025
to get this bug fix: sysgears/webpack-virtual-modules#172

causing this, unclear when:
```
TypeError: Cannot read properties of null (reading 'fileWatchers')
```
serhalp added a commit to gatsbyjs/gatsby that referenced this pull request Aug 15, 2025
to get this bug fix: sysgears/webpack-virtual-modules#172

causing this, unclear when:
```
TypeError: Cannot read properties of null (reading 'fileWatchers')
```
serhalp added a commit to gatsbyjs/gatsby that referenced this pull request Aug 19, 2025
* test(gatsby-cli): tolerate varying stack traces

* test: import mocked module the same way it's used

This mismatch stopped working after node 18.

* test: use more specific snapshot for res headers

Some of the connection header defaults depend on the node.js major version

wip this is a fix for the headers one

* test: skip unstable GC tests on node 20+

* test: adjust assertion to work on node 22 locally

See inline comment

* test: fix sharp build error in lmdb-regeneration fixture

The `lmdb-regeneration` integration test fixture had an `.npmrc` file forcing all packages to be
built from source, which caused sharp to fail compilation in Node 20+ CircleCI images (Ubuntu Noble)
due to... who knows what, honestly.

Since this fixture specifically needs lmdb built from source but not sharp (it's testing something
specific to this), this replaces the global `.npmrc` `build-from-source` flag with a targeted
`postinstall` script that only rebuilds lmdb from source, allowing sharp to use prebuilt binaries.

This resolves the "cannot find -l:libvips-cpp.so.42" linker errors that were blocking integration
tests on Node 20+ while preserving the fixture's intended lmdb compilation testing.

AFAICT this was... basically the only solution. Whew

* fix(gatsby): bump `webpack-virtual-modules`

to get this bug fix: sysgears/webpack-virtual-modules#172

causing this, unclear when:
```
TypeError: Cannot read properties of null (reading 'fileWatchers')
```

* ci: test against Node 18, 20, 22

- run unit tests against all three
- run integration tests against earliest and latest
- make sure to separate npm cache by node version

* ci: test against node 22.13 to avoid bug for now
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants