From ffbec4c5e56496ad8169415e9b4a93ba47c18398 Mon Sep 17 00:00:00 2001 From: Peter van der Zee <209817+pvdz@users.noreply.github.com> Date: Tue, 27 Oct 2020 11:10:24 +0100 Subject: [PATCH] perf(gatsby-source-filesystem): dont JSON parse/stringify the node (#27597) * perf(gatsby-source-filesystem): dont JSON parse/stringify the node * Explicit serialization, remove redundant keys (they are splat anyways) * Explicitly splat stats object * Add a snapshot test * Attempt to make the test more generic so it works on Windows too --- .../src/__tests__/create-file-node.js | 125 +++++++++++++++++- .../src/create-file-node.js | 73 ++++++---- 2 files changed, 168 insertions(+), 30 deletions(-) diff --git a/packages/gatsby-source-filesystem/src/__tests__/create-file-node.js b/packages/gatsby-source-filesystem/src/__tests__/create-file-node.js index 3224b1ad833c4..cf3da21806a17 100644 --- a/packages/gatsby-source-filesystem/src/__tests__/create-file-node.js +++ b/packages/gatsby-source-filesystem/src/__tests__/create-file-node.js @@ -1,11 +1,48 @@ const path = require(`path`) const { createFileNode } = require(`../create-file-node`) +const fs = require(`fs-extra`) + +const fsStatBak = fs.stat // FIXME: This test needs to not use snapshots because of file differences // and locations across users and CI systems describe(`create-file-node`, () => { - it(`creates a file node`, () => { + beforeEach(() => { + // If this breaks, note that the actual values here are not relevant. They just need to be mocked because + // otherwise the tests change due to changing timestamps. The returned object should mimic the real fs.stat + // Note: async tests should run in serial so this mock should not cause cross test polution on this thread. + fs.stat = jest.fn().mockResolvedValue( + Promise.resolve({ + isDirectory() { + return false + }, + dev: 123456, + mode: 123456, + nlink: 123456, + uid: 123456, + rdev: 123456, + blksize: 123456, + ino: 123456, + size: 123456, + blocks: 123456, + atimeMs: 123456, + mtimeMs: 123456, + ctimeMs: 123456, + birthtimeMs: 123456, + atime: new Date(123456), + mtime: new Date(123456), + ctime: new Date(123456), + birthtime: new Date(123456), + }) + ) + }) + + afterEach(() => { + fs.stat = fsStatBak + }) + + it(`creates a file node`, async () => { const createNodeId = jest.fn() createNodeId.mockReturnValue(`uuid-from-gatsby`) return createFileNode( @@ -14,4 +51,90 @@ describe(`create-file-node`, () => { {} ) }) + + it(`records the shape of the node`, async () => { + const dname = fs.mkdtempSync(`gatsby-create-file-node-test`).trim() + try { + const fname = path.join(dname, `f`) + console.log(dname, fname) + fs.writeFileSync(fname, `data`) + try { + const createNodeId = jest.fn() + createNodeId.mockReturnValue(`uuid-from-gatsby`) + + const node = await createFileNode(fname, createNodeId, {}) + + // Sanitize all filenames + Object.keys(node).forEach(key => { + if (typeof node[key] === `string`) { + node[key] = node[key].replace(new RegExp(dname, `g`), ``) + node[key] = node[key].replace(new RegExp(fname, `g`), ``) + } + }) + Object.keys(node.internal).forEach(key => { + if (typeof node.internal[key] === `string`) { + node.internal[key] = node.internal[key].replace( + new RegExp(dname, `g`), + `` + ) + node.internal[key] = node.internal[key].replace( + new RegExp(fname, `g`), + `` + ) + } + }) + + // Note: this snapshot should update if the mock above is changed + expect(node).toMatchInlineSnapshot(` + Object { + "absolutePath": "/f", + "accessTime": "1970-01-01T00:02:03.456Z", + "atime": "1970-01-01T00:02:03.456Z", + "atimeMs": 123456, + "base": "f", + "birthTime": "1970-01-01T00:02:03.456Z", + "birthtime": "1970-01-01T00:02:03.456Z", + "birthtimeMs": 123456, + "blksize": 123456, + "blocks": 123456, + "changeTime": "1970-01-01T00:02:03.456Z", + "children": Array [], + "ctime": "1970-01-01T00:02:03.456Z", + "ctimeMs": 123456, + "dev": 123456, + "dir": "", + "ext": "", + "extension": "", + "id": "uuid-from-gatsby", + "ino": 123456, + "internal": Object { + "contentDigest": "8d777f385d3dfec8815d20f7496026dc", + "description": "File \\"/f\\"", + "mediaType": "application/octet-stream", + "type": "File", + }, + "mode": 123456, + "modifiedTime": "1970-01-01T00:02:03.456Z", + "mtime": "1970-01-01T00:02:03.456Z", + "mtimeMs": 123456, + "name": "f", + "nlink": 123456, + "parent": null, + "prettySize": "123 kB", + "rdev": 123456, + "relativeDirectory": "", + "relativePath": "/f", + "root": "", + "size": 123456, + "sourceInstanceName": "__PROGRAMMATIC__", + "uid": 123456, + } + `) + } finally { + fs.unlinkSync(fname) + } + } finally { + fs.rmdirSync(dname) + } + }) }) diff --git a/packages/gatsby-source-filesystem/src/create-file-node.js b/packages/gatsby-source-filesystem/src/create-file-node.js index 12e72bc7eb1aa..3e9730fad6593 100644 --- a/packages/gatsby-source-filesystem/src/create-file-node.js +++ b/packages/gatsby-source-filesystem/src/create-file-node.js @@ -45,33 +45,48 @@ exports.createFileNode = async ( } } - // Stringify date objects. - return JSON.parse( - JSON.stringify({ - // Don't actually make the File id the absolute path as otherwise - // people will use the id for that and ids shouldn't be treated as - // useful information. - id: createNodeId(pathToFile), - children: [], - parent: null, - internal, - sourceInstanceName: pluginOptions.name || `__PROGRAMMATIC__`, - absolutePath: slashedFile.absolutePath, - relativePath: slash( - path.relative( - pluginOptions.path || process.cwd(), - slashedFile.absolutePath - ) - ), - extension: slashedFile.ext.slice(1).toLowerCase(), - size: stats.size, - prettySize: prettyBytes(stats.size), - modifiedTime: stats.mtime, - accessTime: stats.atime, - changeTime: stats.ctime, - birthTime: stats.birthtime, - ...slashedFile, - ...stats, - }) - ) + return { + // Don't actually make the File id the absolute path as otherwise + // people will use the id for that and ids shouldn't be treated as + // useful information. + id: createNodeId(pathToFile), + children: [], + parent: null, + internal, + sourceInstanceName: pluginOptions.name || `__PROGRAMMATIC__`, + relativePath: slash( + path.relative( + pluginOptions.path || process.cwd(), + slashedFile.absolutePath + ) + ), + extension: slashedFile.ext.slice(1).toLowerCase(), + prettySize: prettyBytes(stats.size), + modifiedTime: stats.mtime.toJSON(), + accessTime: stats.atime.toJSON(), + changeTime: stats.ctime.toJSON(), + birthTime: stats.birthtime.toJSON(), + // Note: deprecate splatting the slashedFile object + // Note: the object may contain different properties depending on File or Dir + ...slashedFile, + // TODO: deprecate copying the entire object + // Note: not splatting for perf reasons (make sure Date objects are serialized) + dev: stats.dev, + mode: stats.mode, + nlink: stats.nlink, + uid: stats.uid, + rdev: stats.rdev, + blksize: stats.blksize, + ino: stats.ino, + size: stats.size, + blocks: stats.blocks, + atimeMs: stats.atimeMs, + mtimeMs: stats.mtimeMs, + ctimeMs: stats.ctimeMs, + birthtimeMs: stats.birthtimeMs, + atime: stats.atime.toJSON(), + mtime: stats.mtime.toJSON(), + ctime: stats.ctime.toJSON(), + birthtime: stats.birthtime.toJSON(), + } }