fix: add missing fs method rewrites to handle fetchRemoteFile in dsg/ssr engine #38822
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ import { promisify } from "util" | |
|
|
||
| import type { IGatsbyPage } from "../../internal" | ||
| import type { ISSRData } from "./entry" | ||
| import { link, rewritableMethods as linkRewritableMethods } from "linkfs" | ||
|
|
||
| const cdnDatastore = `%CDN_DATASTORE_PATH%` | ||
| const PATH_PREFIX = `%PATH_PREFIX%` | ||
|
|
@@ -30,7 +30,7 @@ function setupFsWrapper(): string { | |
| global.__GATSBY.root = TEMP_DIR | ||
|
|
||
| // TODO: don't hardcode this | ||
| const cacheDir = `${process.cwd()}/.cache` | ||
|
|
||
| // we need to rewrite fs | ||
| const rewrites = [ | ||
|
|
@@ -47,25 +47,132 @@ function setupFsWrapper(): string { | |
| to: TEMP_CACHE_DIR, | ||
| rewrites, | ||
| }) | ||
|
|
||
| // copied from https://github.com/streamich/linkfs/blob/master/src/index.ts#L126-L142 | ||
| function mapPathUsingRewrites(fsPath: fs.PathLike): string { | ||
| let filename = path.resolve(String(fsPath)) | ||
| for (const [from, to] of rewrites) { | ||
| if (filename.indexOf(from) === 0) { | ||
| const rootRegex = /(?:^[a-zA-Z]:\\$)|(?:^\/$)/ // C:\ vs / | ||
| const isRoot = from.match(rootRegex) | ||
| const baseRegex = `^(` + from.replace(/\\/g, `\\\\`) + `)` | ||
|
|
||
| if (isRoot) { | ||
| const regex = new RegExp(baseRegex) | ||
| filename = filename.replace(regex, () => to + path.sep) | ||
| } else { | ||
| const regex = new RegExp(baseRegex + `(\\\\|/|$)`) | ||
| filename = filename.replace(regex, (_match, _p1, p2) => to + p2) | ||
| } | ||
| } | ||
| } | ||
| return filename | ||
| } | ||
|
|
||
| function applyRename< | ||
| T = typeof import("fs") | typeof import("fs").promises | ||
| >(fsToRewrite: T, lfs: T, method: "rename" | "renameSync"): void { | ||
| const original = fsToRewrite[method] | ||
| if (original) { | ||
| // @ts-ignore - complains about __promisify__ which doesn't actually exist in runtime | ||
| lfs[method] = ( | ||
| ...args: Parameters<typeof import("fs")["rename" | "renameSync"]> | ||
| ): ReturnType<typeof import("fs")["rename" | "renameSync"]> => { | ||
| args[0] = mapPathUsingRewrites(args[0]) | ||
| args[1] = mapPathUsingRewrites(args[1]) | ||
| // @ts-ignore - can't decide which signature this is, but we just preserve the original | ||
| // signature here and mostly care about first 2 arguments being PathLike | ||
| return original.apply(fsToRewrite, args) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // linkfs doesn't handle following methods, so we need to add them manually | ||
| linkRewritableMethods.push(`createWriteStream`, `createReadStream`, `rm`) | ||
|
|
||
| function createLinkedFS< | ||
| T = typeof import("fs") | typeof import("fs").promises | ||
| >(fsToRewrite: T): T { | ||
| const linkedFS = link(fsToRewrite, rewrites) as T | ||
|
|
||
| // linkfs doesn't handle `to` argument in `rename` and `renameSync` methods | ||
| applyRename(fsToRewrite, linkedFS, `rename`) | ||
| applyRename(fsToRewrite, linkedFS, `renameSync`) | ||
|
|
||
| return linkedFS | ||
| } | ||
|
|
||
| // Alias the cache dir paths to the temp dir | ||
| const lfs = createLinkedFS(fs) | ||
|
|
||
| // linkfs doesn't pass across the `native` prop, which graceful-fs needs | ||
| for (const key in lfs) { | ||
| if (Object.hasOwnProperty.call(fs[key], `native`)) { | ||
| lfs[key].native = fs[key].native | ||
| } | ||
| } | ||
| // 'promises' is not initially linked within the 'linkfs' | ||
| // package, and is needed by underlying Gatsby code (the | ||
| // @graphql-tools/code-file-loader) | ||
| lfs.promises = createLinkedFS(fs.promises) | ||
|
|
||
| const originalWritesStream = fs.WriteStream | ||
| function LinkedWriteStream( | ||
| this: fs.WriteStream, | ||
| ...args: Parameters<(typeof fs)["createWriteStream"]> | ||
| ): fs.WriteStream { | ||
| args[0] = mapPathUsingRewrites(args[0]) | ||
| args[1] = | ||
| typeof args[1] === `string` | ||
| ? { | ||
| flags: args[1], | ||
| // @ts-ignore there is `fs` property in options doh (https://nodejs.org/api/fs.html#fscreatewritestreampath-options) | ||
| fs: lfs, | ||
| } | ||
| : { | ||
| ...(args[1] || {}), | ||
| // @ts-ignore there is `fs` property in options doh (https://nodejs.org/api/fs.html#fscreatewritestreampath-options) | ||
| fs: lfs, | ||
| } | ||
|
|
||
| // @ts-ignore TS doesn't like extending prototype "classes" | ||
| return originalWritesStream.apply(this, args) | ||
| } | ||
| LinkedWriteStream.prototype = Object.create(originalWritesStream.prototype) | ||
| // @ts-ignore TS doesn't like extending prototype "classes" | ||
| lfs.WriteStream = LinkedWriteStream | ||
|
|
||
| const originalReadStream = fs.ReadStream | ||
| function LinkedReadStream( | ||
| this: fs.ReadStream, | ||
| ...args: Parameters<(typeof fs)["createReadStream"]> | ||
| ): fs.ReadStream { | ||
| args[0] = mapPathUsingRewrites(args[0]) | ||
| args[1] = | ||
| typeof args[1] === `string` | ||
| ? { | ||
| flags: args[1], | ||
| // @ts-ignore there is `fs` property in options doh (https://nodejs.org/api/fs.html#fscreatewritestreampath-options) | ||
| fs: lfs, | ||
| } | ||
| : { | ||
| ...(args[1] || {}), | ||
| // @ts-ignore there is `fs` property in options doh (https://nodejs.org/api/fs.html#fscreatewritestreampath-options) | ||
| fs: lfs, | ||
| } | ||
|
|
||
| // @ts-ignore TS doesn't like extending prototype "classes" | ||
| return originalReadStream.apply(this, args) | ||
| } | ||
| LinkedReadStream.prototype = Object.create(originalReadStream.prototype) | ||
| // @ts-ignore TS doesn't like extending prototype "classes" | ||
| lfs.ReadStream = LinkedReadStream | ||
|
|
||
| const dbPath = path.join(TEMP_CACHE_DIR, `data`, `datastore`) | ||
|
|
||
|
There was a problem hiding this comment. Seems like there's some repetition here with extending fs.WriteStream and fs.ReadStream, you could consider extracting/reusing. There was a problem hiding this comment. Yes, it's basically the same thing twice (just once for There was a problem hiding this comment. Doing There was a problem hiding this comment. Got it, I'm ok leaving as is. |
||
| // Gatsby uses this instead of fs if present | ||
| // eslint-disable-next-line no-underscore-dangle | ||
| // @ts-ignore __promisify__ stuff | ||
| global._fsWrapper = lfs | ||
|
|
||
| if (!cdnDatastore) { | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 test was disabled because of problems that this PR is fixing. In context of this test those problems weren't fatal, however it meant that image placeholders were just not working.
The fix in this PR also solves other problems than just placeholders, but this was existing test that already we had