From 4f3e05039c9bbefc8d9e98efb0f815eec0a8fea9 Mon Sep 17 00:00:00 2001 From: Dustin Schau Date: Wed, 2 Jan 2019 17:52:57 -0600 Subject: [PATCH 1/4] fix(gatsby): correctly replace static query content in themes This PR (correctly) replaces static query content in themes. It does this with a probably naive algorithm, where it checks the plugins array, and if a plugins path matches (gatsby-theme-), that plugin will be added to the query compiler list. It also slightly cleans up some of the code in the query compiler and adds a few tests Notes: - I validated with this repo -> https://github.com/DSchau/gatsby-theme-static-query-repro - Can't really tie into webpack (e.g. to detect a custom loader targeting node_modules/whatever-custom-component) because we use webpack-merge to persist into redux state, which gets translated to an empty object; although I would like to find some solution to non-theme packages in node_modules Feedback welcome, cc @ChristopherBiscardi @jbolda --- .../query-runner/__tests__/query-compiler.js | 84 +++++++++++++++++++ .../query-runner/query-compiler.js | 47 +++++++---- 2 files changed, 115 insertions(+), 16 deletions(-) create mode 100644 packages/gatsby/src/internal-plugins/query-runner/__tests__/query-compiler.js diff --git a/packages/gatsby/src/internal-plugins/query-runner/__tests__/query-compiler.js b/packages/gatsby/src/internal-plugins/query-runner/__tests__/query-compiler.js new file mode 100644 index 0000000000000..ee7d5804cd9d5 --- /dev/null +++ b/packages/gatsby/src/internal-plugins/query-runner/__tests__/query-compiler.js @@ -0,0 +1,84 @@ +jest.mock(`glob`, () => { + const sync = jest.fn().mockImplementation(() => []) + return { + sync, + } +}) +const path = require(`path`) +const glob = require(`glob`) +const { resolveThemes, Runner } = require(`../query-compiler`) + +const base = path.resolve(``) + +describe(`Runner`, () => { + beforeEach(() => { + glob.sync.mockClear() + }) + + it(`returns a file parser instance`, async () => { + const runner = new Runner(base, [], {}) + + const parser = await runner.parseEverything() + + expect(parser).toEqual(new Map()) + }) + + describe(`expected directories`, () => { + it(`compiles src directory`, async () => { + const runner = new Runner(base, [], {}) + + await runner.parseEverything() + + expect(glob.sync).toHaveBeenCalledWith( + expect.stringContaining(path.join(base, `src`)), + expect.any(Object) + ) + }) + + it(`compiles fragments directory`, async () => { + const runner = new Runner(base, [], {}) + + await runner.parseEverything() + + expect(glob.sync).toHaveBeenCalledWith( + expect.stringContaining(path.join(base, `src`)), + expect.any(Object) + ) + }) + + it(`compiles themes directory(s)`, async () => { + const theme = `gatsby-theme-whatever` + const runner = new Runner(base, [path.join(`node_modules`, theme)], {}) + + await runner.parseEverything() + + expect(glob.sync).toHaveBeenCalledWith( + expect.stringContaining(path.join(base, `node_modules`, theme)), + expect.any(Object) + ) + }) + }) +}) + +describe(`resolveThemes`, () => { + it(`returns empty array if zero themes detected`, () => { + ;[ + [], + [{ resolve: path.join(base, `gatsby-plugin-whatever`) }], + undefined, + ].forEach(testRun => { + expect(resolveThemes(base, testRun)).toEqual([]) + }) + }) + + it(`returns plugins matching gatsby-theme prefix`, () => { + const theme = `gatsby-theme-example` + expect( + resolveThemes(base, [ + { + resolve: path.join(base, `gatsby-theme-example`), + }, + ]) + ).toEqual([theme]) + }) +}) diff --git a/packages/gatsby/src/internal-plugins/query-runner/query-compiler.js b/packages/gatsby/src/internal-plugins/query-runner/query-compiler.js index e37e85e3829ab..6f3cd82dd614a 100644 --- a/packages/gatsby/src/internal-plugins/query-runner/query-compiler.js +++ b/packages/gatsby/src/internal-plugins/query-runner/query-compiler.js @@ -64,15 +64,24 @@ const validationRules = [ let lastRunHadErrors = null const overlayErrorID = `graphql-compiler` +const resolveThemes = (base, plugins = []) => + plugins.reduce((merged, plugin) => { + if (plugin.resolve.includes(`gatsby-theme-`)) { + merged.push(path.relative(base, plugin.resolve)) + } + return merged + }, []) + class Runner { - baseDir: string + base: string + additional: string[] schema: GraphQLSchema errors: string[] fragmentsDir: string - constructor(baseDir: string, fragmentsDir: string, schema: GraphQLSchema) { - this.baseDir = baseDir - this.fragmentsDir = fragmentsDir + constructor(base: string, additional: string[], schema: GraphQLSchema) { + this.base = base + this.additional = additional this.schema = schema } @@ -91,14 +100,20 @@ class Runner { } async parseEverything() { - // FIXME: this should all use gatsby's configuration to determine parsable - // files (and how to parse them) - let files = glob.sync(`${this.fragmentsDir}/**/*.+(t|j)s?(x)`, { - nodir: true, - }) - files = files.concat( - glob.sync(`${this.baseDir}/**/*.+(t|j)s?(x)`, { nodir: true }) - ) + const filesRegex = `/**/*.+(t|j)s?(x)` + let files = [`${this.base}/src`, `${this.base}/.cache/fragments`] + .concat( + this.additional.map(additional => path.join(this.base, additional)) + ) + .reduce( + (merged, base) => + merged.concat( + glob.sync(`${base}${filesRegex}`, { + nodir: true, + }) + ), + [] + ) files = files.filter(d => !d.match(/\.d\.ts$/)) files = files.map(normalize) @@ -217,14 +232,14 @@ class Runner { return compiledNodes } } -export { Runner } +export { Runner, resolveThemes } export default async function compile(): Promise> { - const { program, schema } = store.getState() + const { program, schema, plugins } = store.getState() const runner = new Runner( - `${program.directory}/src`, - `${program.directory}/.cache/fragments`, + program.directory, + resolveThemes(program.directory, plugins), schema ) From 2ec498c2e5adf3d9274ac1c0edef15bbaf1583d8 Mon Sep 17 00:00:00 2001 From: Dustin Schau Date: Wed, 2 Jan 2019 17:58:52 -0600 Subject: [PATCH 2/4] chore: add a scoped package test --- .../query-runner/__tests__/query-compiler.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/gatsby/src/internal-plugins/query-runner/__tests__/query-compiler.js b/packages/gatsby/src/internal-plugins/query-runner/__tests__/query-compiler.js index ee7d5804cd9d5..bc61e0eacafb5 100644 --- a/packages/gatsby/src/internal-plugins/query-runner/__tests__/query-compiler.js +++ b/packages/gatsby/src/internal-plugins/query-runner/__tests__/query-compiler.js @@ -81,4 +81,16 @@ describe(`resolveThemes`, () => { ]) ).toEqual([theme]) }) + + it(`handles scoped packages`, () => { + const theme = `@dschau/gatsby-theme-example` + + expect( + resolveThemes(base, [ + { + resolve: path.join(base, theme), + }, + ]) + ).toEqual([theme]) + }) }) From 90419a45d21679582327024afc1c078aaa746d11 Mon Sep 17 00:00:00 2001 From: Dustin Schau Date: Thu, 3 Jan 2019 10:15:25 -0600 Subject: [PATCH 3/4] refactor(gatsby): just use the resolved path, don't make relative --- .../query-runner/__tests__/query-compiler.js | 16 ++++++++++------ .../query-runner/query-compiler.js | 19 +++++++------------ 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/packages/gatsby/src/internal-plugins/query-runner/__tests__/query-compiler.js b/packages/gatsby/src/internal-plugins/query-runner/__tests__/query-compiler.js index bc61e0eacafb5..6da385f8dde73 100644 --- a/packages/gatsby/src/internal-plugins/query-runner/__tests__/query-compiler.js +++ b/packages/gatsby/src/internal-plugins/query-runner/__tests__/query-compiler.js @@ -48,7 +48,11 @@ describe(`Runner`, () => { it(`compiles themes directory(s)`, async () => { const theme = `gatsby-theme-whatever` - const runner = new Runner(base, [path.join(`node_modules`, theme)], {}) + const runner = new Runner( + base, + [path.join(base, `node_modules`, theme)], + {} + ) await runner.parseEverything() @@ -67,30 +71,30 @@ describe(`resolveThemes`, () => { [{ resolve: path.join(base, `gatsby-plugin-whatever`) }], undefined, ].forEach(testRun => { - expect(resolveThemes(base, testRun)).toEqual([]) + expect(resolveThemes(testRun)).toEqual([]) }) }) it(`returns plugins matching gatsby-theme prefix`, () => { const theme = `gatsby-theme-example` expect( - resolveThemes(base, [ + resolveThemes([ { resolve: path.join(base, `gatsby-theme-example`), }, ]) - ).toEqual([theme]) + ).toEqual([expect.stringContaining(theme)]) }) it(`handles scoped packages`, () => { const theme = `@dschau/gatsby-theme-example` expect( - resolveThemes(base, [ + resolveThemes([ { resolve: path.join(base, theme), }, ]) - ).toEqual([theme]) + ).toEqual([expect.stringContaining(theme.split(`/`).join(path.sep))]) }) }) diff --git a/packages/gatsby/src/internal-plugins/query-runner/query-compiler.js b/packages/gatsby/src/internal-plugins/query-runner/query-compiler.js index 6f3cd82dd614a..e0bccb9fb874f 100644 --- a/packages/gatsby/src/internal-plugins/query-runner/query-compiler.js +++ b/packages/gatsby/src/internal-plugins/query-runner/query-compiler.js @@ -64,10 +64,10 @@ const validationRules = [ let lastRunHadErrors = null const overlayErrorID = `graphql-compiler` -const resolveThemes = (base, plugins = []) => +const resolveThemes = (plugins = []) => plugins.reduce((merged, plugin) => { if (plugin.resolve.includes(`gatsby-theme-`)) { - merged.push(path.relative(base, plugin.resolve)) + merged.push(plugin.resolve) } return merged }, []) @@ -102,13 +102,11 @@ class Runner { async parseEverything() { const filesRegex = `/**/*.+(t|j)s?(x)` let files = [`${this.base}/src`, `${this.base}/.cache/fragments`] - .concat( - this.additional.map(additional => path.join(this.base, additional)) - ) + .concat(this.additional.map(additional => `${additional}/src`)) .reduce( - (merged, base) => + (merged, folderPath) => merged.concat( - glob.sync(`${base}${filesRegex}`, { + glob.sync(`${folderPath}${filesRegex}`, { nodir: true, }) ), @@ -235,13 +233,10 @@ class Runner { export { Runner, resolveThemes } export default async function compile(): Promise> { + // TODO: swap plugins to themes const { program, schema, plugins } = store.getState() - const runner = new Runner( - program.directory, - resolveThemes(program.directory, plugins), - schema - ) + const runner = new Runner(program.directory, resolveThemes(plugins), schema) const queries = await runner.compileAll() From 3b65605158498408924a858e54dc7dbd0523afd7 Mon Sep 17 00:00:00 2001 From: Dustin Schau Date: Thu, 3 Jan 2019 11:23:46 -0600 Subject: [PATCH 4/4] fix: hopefully fix windows issues (fingers crossed emoji) --- .../internal-plugins/query-runner/query-compiler.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/gatsby/src/internal-plugins/query-runner/query-compiler.js b/packages/gatsby/src/internal-plugins/query-runner/query-compiler.js index e0bccb9fb874f..002a888a50419 100644 --- a/packages/gatsby/src/internal-plugins/query-runner/query-compiler.js +++ b/packages/gatsby/src/internal-plugins/query-runner/query-compiler.js @@ -100,13 +100,16 @@ class Runner { } async parseEverything() { - const filesRegex = `/**/*.+(t|j)s?(x)` - let files = [`${this.base}/src`, `${this.base}/.cache/fragments`] - .concat(this.additional.map(additional => `${additional}/src`)) + const filesRegex = path.join(`/**`, `*.+(t|j)s?(x)`) + let files = [ + path.join(this.base, `src`), + path.join(this.base, `.cache`, `fragments`), + ] + .concat(this.additional.map(additional => path.join(additional, `src`))) .reduce( (merged, folderPath) => merged.concat( - glob.sync(`${folderPath}${filesRegex}`, { + glob.sync(path.join(folderPath, filesRegex), { nodir: true, }) ),