From c60be721654fb1f737a7ddfea4e33b81749dfaa4 Mon Sep 17 00:00:00 2001 From: phacks Date: Wed, 5 Dec 2018 10:41:48 +0100 Subject: [PATCH 1/3] Use `recursive-readdir` instead of `recursive-readdir-synchronous` --- .../gatsby-remark-code-repls/package.json | 2 +- .../src/__tests__/gatsby-node.js | 126 +++++++++++------- .../src/gatsby-node.js | 79 +++++------ yarn.lock | 34 ++--- 4 files changed, 125 insertions(+), 116 deletions(-) diff --git a/packages/gatsby-remark-code-repls/package.json b/packages/gatsby-remark-code-repls/package.json index 9f2488260d9ee..f24285a2cec23 100644 --- a/packages/gatsby-remark-code-repls/package.json +++ b/packages/gatsby-remark-code-repls/package.json @@ -10,7 +10,7 @@ "@babel/runtime": "^7.0.0", "lz-string": "^1.4.4", "normalize-path": "^2.1.1", - "recursive-readdir-synchronous": "^0.0.3", + "recursive-readdir": "^2.2.2", "unist-util-map": "^1.0.3", "urijs": "^1.19.0" }, diff --git a/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js b/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js index 76f6d1b1a0e55..f55e419db3b0d 100644 --- a/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js +++ b/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js @@ -4,10 +4,10 @@ jest.mock(`fs`, () => { readFileSync: jest.fn(), } }) -jest.mock(`recursive-readdir-synchronous`, () => jest.fn()) +jest.mock(`recursive-readdir`, () => jest.fn()) const fs = require(`fs`) -const recursiveReaddir = require(`recursive-readdir-synchronous`) +const readdir = require(`recursive-readdir`) const { OPTION_DEFAULT_HTML, @@ -30,20 +30,17 @@ describe(`gatsby-remark-code-repls`, () => { fs.readFileSync.mockReset() fs.readFileSync.mockReturnValue(`const foo = "bar";`) - recursiveReaddir.mockReset() - recursiveReaddir.mockReturnValue([`file.js`]) + readdir.mockReset() + readdir.mockResolvedValue([`file.js`]) createPage.mockReset() }) describe(`gatsby-node`, () => { - it(`should iterate over all JavaScript files in the examples directory`, () => { - recursiveReaddir.mockReturnValue([ - `root-file.js`, - `path/to/nested/file.jsx`, - ]) + it(`should iterate over all JavaScript files in the examples directory`, async () => { + readdir.mockResolvedValue([`root-file.js`, `path/to/nested/file.jsx`]) - createPages(createPagesParams) + await createPages(createPagesParams) expect(fs.readFileSync).toHaveBeenCalledTimes(2) expect(fs.readFileSync).toHaveBeenCalledWith(`root-file.js`, `utf8`) @@ -53,52 +50,51 @@ describe(`gatsby-remark-code-repls`, () => { ) }) - it(`should ignore non JavaScript files in the examples directory`, () => { - recursiveReaddir.mockReturnValue([`javascript.js`, `not-javascript.html`]) + it(`should ignore non JavaScript files in the examples directory`, async () => { + readdir.mockResolvedValue([`javascript.js`, `not-javascript.html`]) - createPages(createPagesParams) + await createPages(createPagesParams) expect(fs.readFileSync).toHaveBeenCalledTimes(1) expect(fs.readFileSync).toHaveBeenCalledWith(`javascript.js`, `utf8`) }) - it(`should error if provided an invalid examples directory`, () => { + it(`should error if provided an invalid examples directory`, async () => { fs.existsSync.mockReturnValue(false) - expect(() => createPages(createPagesParams)).toThrow( - `Invalid REPL directory specified: "REPL/"` - ) + try { + await createPages(createPagesParams) + } catch (err) { + expect(err).toEqual(Error(`Invalid REPL directory specified: "REPL/"`)) + } }) - it(`should warn about an empty examples directory`, () => { - recursiveReaddir.mockReturnValue([]) + it(`should warn about an empty examples directory`, async () => { + readdir.mockResolvedValue([]) spyOn(console, `warn`) // eslint-disable-line no-undef - createPages(createPagesParams) + await createPages(createPagesParams) expect(console.warn).toHaveBeenCalledWith( `Specified REPL directory "REPL/" contains no files` ) }) - it(`should create redirect pages for the code in each example file`, () => { - recursiveReaddir.mockReturnValue([ - `root-file.js`, - `path/to/nested/file.jsx`, - ]) + it(`should create redirect pages for the code in each example file`, async () => { + readdir.mockResolvedValue([`root-file.js`, `path/to/nested/file.jsx`]) - createPages(createPagesParams) + await createPages(createPagesParams) expect(createPage).toHaveBeenCalledTimes(2) expect(createPage.mock.calls[0][0].path).toContain(`root-file`) expect(createPage.mock.calls[1][0].path).toContain(`path/to/nested/file`) }) - it(`should use a default redirect template`, () => { - recursiveReaddir.mockReturnValue([`file.js`]) + it(`should use a default redirect template`, async () => { + readdir.mockResolvedValue([`file.js`]) - createPages(createPagesParams) + await createPages(createPagesParams) expect(createPage).toHaveBeenCalledTimes(1) expect(createPage.mock.calls[0][0].component).toContain( @@ -106,27 +102,55 @@ describe(`gatsby-remark-code-repls`, () => { ) }) - it(`should use a specified redirect template override`, () => { - recursiveReaddir.mockReturnValue([`file.js`]) + it(`should use a specified redirect template override`, async () => { + readdir.mockResolvedValue([`file.js`]) - createPages(createPagesParams, { redirectTemplate: `foo/bar.js` }) + await createPages(createPagesParams, { redirectTemplate: `foo/bar.js` }) expect(createPage).toHaveBeenCalledTimes(1) expect(createPage.mock.calls[0][0].component).toContain(`foo/bar.js`) }) - it(`should error if an invalid redirect template is specified`, () => { + it(`should error if an invalid redirect template is specified`, async () => { fs.existsSync.mockImplementation(path => path !== `foo/bar.js`) - expect(() => - createPages(createPagesParams, { redirectTemplate: `foo/bar.js` }) - ).toThrow(`Invalid REPL redirectTemplate specified`) + try { + await createPages(createPagesParams, { redirectTemplate: `foo/bar.js` }) + } catch (err) { + expect(err).toEqual( + Error(`Invalid REPL redirectTemplate specified: "foo/bar.js"`) + ) + } + }) + + it(`should propagate any Error from recursive-readdir`, async () => { + const error = TypeError(`glob pattern string required`) + + readdir.mockRejectedValue(error) + + try { + await createPages(createPagesParams) + } catch (err) { + expect(err).toBe(error) + } + }) + + it(`should wrap any non-error rejection from recursive-readdir in an Error instance`, async () => { + const rejectionMessage = `an error message` + + readdir.mockRejectedValue(rejectionMessage) + + try { + await createPages(createPagesParams) + } catch (err) { + expect(err).toEqual(Error(rejectionMessage)) + } }) - it(`should not load any external packages by default`, () => { - recursiveReaddir.mockReturnValue([`file.js`]) + it(`should not load any external packages by default`, async () => { + readdir.mockResolvedValue([`file.js`]) - createPages(createPagesParams) + await createPages(createPagesParams) const { js_external } = JSON.parse( createPage.mock.calls[0][0].context.payload @@ -135,10 +159,10 @@ describe(`gatsby-remark-code-repls`, () => { expect(js_external).toBe(``) }) - it(`should load custom externals if specified`, () => { - recursiveReaddir.mockReturnValue([`file.js`]) + it(`should load custom externals if specified`, async () => { + readdir.mockResolvedValue([`file.js`]) - createPages(createPagesParams, { externals: [`foo.js`, `bar.js`] }) + await createPages(createPagesParams, { externals: [`foo.js`, `bar.js`] }) const { js_external } = JSON.parse( createPage.mock.calls[0][0].context.payload @@ -148,10 +172,10 @@ describe(`gatsby-remark-code-repls`, () => { expect(js_external).toContain(`bar.js`) }) - it(`should inject the required prop-types for the Codepen prefill API`, () => { - recursiveReaddir.mockReturnValue([`file.js`]) + it(`should inject the required prop-types for the Codepen prefill API`, async () => { + readdir.mockResolvedValue([`file.js`]) - createPages(createPagesParams) + await createPages(createPagesParams) const { action, payload } = createPage.mock.calls[0][0].context @@ -159,20 +183,20 @@ describe(`gatsby-remark-code-repls`, () => { expect(payload).toBeTruthy() }) - it(`should render default HTML for index page if no override specified`, () => { - recursiveReaddir.mockReturnValue([`file.js`]) + it(`should render default HTML for index page if no override specified`, async () => { + readdir.mockResolvedValue([`file.js`]) - createPages(createPagesParams, {}) + await createPages(createPagesParams, {}) const { html } = JSON.parse(createPage.mock.calls[0][0].context.payload) expect(html).toBe(OPTION_DEFAULT_HTML) }) - it(`should support custom, user-defined HTML for index page`, () => { - recursiveReaddir.mockReturnValue([`file.js`]) + it(`should support custom, user-defined HTML for index page`, async () => { + readdir.mockResolvedValue([`file.js`]) - createPages(createPagesParams, { html: `` }) + await createPages(createPagesParams, { html: `` }) const { html } = JSON.parse(createPage.mock.calls[0][0].context.payload) diff --git a/packages/gatsby-remark-code-repls/src/gatsby-node.js b/packages/gatsby-remark-code-repls/src/gatsby-node.js index e17c71e009317..ebada4a0c7ea3 100644 --- a/packages/gatsby-remark-code-repls/src/gatsby-node.js +++ b/packages/gatsby-remark-code-repls/src/gatsby-node.js @@ -2,7 +2,7 @@ const fs = require(`fs`) const { extname, resolve } = require(`path`) -const recursiveReaddir = require(`recursive-readdir-synchronous`) +const readdir = require(`recursive-readdir`) const normalizePath = require(`normalize-path`) const { @@ -11,7 +11,7 @@ const { OPTION_DEFAULT_REDIRECT_TEMPLATE_PATH, } = require(`./constants`) -exports.createPages = ( +exports.createPages = async ( { actions }, { directory = OPTION_DEFAULT_LINK_TEXT, @@ -36,45 +36,48 @@ exports.createPages = ( ) } - // TODO We could refactor this to use 'recursive-readdir' instead, - // And wrap with Promise.all() to execute createPage() in parallel. - // I'd need to find a way to reliably test error handling though. - const files = recursiveReaddir(directory) + try { + const files = await readdir(directory) + if (files.length === 0) { + console.warn(`Specified REPL directory "${directory}" contains no files`) - if (files.length === 0) { - console.warn(`Specified REPL directory "${directory}" contains no files`) - - return - } + return + } - files.forEach(file => { - if (extname(file) === `.js` || extname(file) === `.jsx`) { - const slug = file - .substring(0, file.length - extname(file).length) - .replace(new RegExp(`^${directory}`), `redirect-to-codepen/`) - const code = fs.readFileSync(file, `utf8`) + files.forEach(file => { + if (extname(file) === `.js` || extname(file) === `.jsx`) { + const slug = file + .substring(0, file.length - extname(file).length) + .replace(new RegExp(`^${directory}`), `redirect-to-codepen/`) + const code = fs.readFileSync(file, `utf8`) - // Codepen configuration. - // https://blog.codepen.io/documentation/api/prefill/ - const action = `https://codepen.io/pen/define` - const payload = JSON.stringify({ - editors: `0010`, - html, - js: code, - js_external: externals.join(`;`), - js_pre_processor: `babel`, - layout: `left`, - }) + // Codepen configuration. + // https://blog.codepen.io/documentation/api/prefill/ + const action = `https://codepen.io/pen/define` + const payload = JSON.stringify({ + editors: `0010`, + html, + js: code, + js_external: externals.join(`;`), + js_pre_processor: `babel`, + layout: `left`, + }) - createPage({ - path: slug, - // Normalize the path so tests pass on Linux + Windows - component: normalizePath(resolve(redirectTemplate)), - context: { - action, - payload, - }, - }) + createPage({ + path: slug, + // Normalize the path so tests pass on Linux + Windows + component: normalizePath(resolve(redirectTemplate)), + context: { + action, + payload, + }, + }) + } + }) + } catch (error) { + if (error instanceof Error) { + throw error } - }) + throw Error(error) + } } diff --git a/yarn.lock b/yarn.lock index df4dbe404e6e6..98d10bdf31290 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12686,11 +12686,6 @@ lpad-align@^1.0.1: longest "^1.0.0" meow "^3.3.0" -lru-cache@2: - version "2.7.3" - resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-2.7.3.tgz#6d4524e8b955f95d4f5b58851ce21dd72fb4e952" - integrity sha1-bUUk6LlV+V1PW1iFHOId1y+06VI= - lru-cache@4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/lru-cache/-/lru-cache-4.0.0.tgz#b5cbf01556c16966febe54ceec0fb4dc90df6c28" @@ -13210,15 +13205,7 @@ minimalistic-crypto-utils@^1.0.0, minimalistic-crypto-utils@^1.0.1: resolved "https://registry.yarnpkg.com/minimalistic-crypto-utils/-/minimalistic-crypto-utils-1.0.1.tgz#f6c00c1c0b082246e5c4d99dfb8c7c083b2b582a" integrity sha1-9sAMHAsIIkblxNmd+4x8CDsrWCo= -minimatch@0.3.0: - version "0.3.0" - resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-0.3.0.tgz#275d8edaac4f1bb3326472089e7949c8394699dd" - integrity sha1-J12O2qxPG7MyZHIInnlJyDlGmd0= - dependencies: - lru-cache "2" - sigmund "~1.0.0" - -"minimatch@2 || 3", minimatch@^3.0.0, minimatch@^3.0.2, minimatch@^3.0.3, minimatch@^3.0.4: +"minimatch@2 || 3", minimatch@3.0.4, minimatch@^3.0.0, minimatch@^3.0.2, minimatch@^3.0.3, minimatch@^3.0.4: version "3.0.4" resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-3.0.4.tgz#5166e286457f03306064be5497e8dbb0c3d32083" integrity sha512-yJHVQEhyqPLUTgt9B83PXu6W3rx4MvvHvSUvToogpwoGDOUQ+yDrR0HRot+yOCdCO7u4hX3pWft6kWBBcqh0UA== @@ -16269,13 +16256,6 @@ rechoir@^0.6.2: dependencies: resolve "^1.1.6" -recursive-readdir-synchronous@^0.0.3: - version "0.0.3" - resolved "https://registry.yarnpkg.com/recursive-readdir-synchronous/-/recursive-readdir-synchronous-0.0.3.tgz#d5e5a096ad56cf9666241c22a30b4f338bb7ed88" - integrity sha1-1eWglq1Wz5ZmJBwiowtPM4u37Yg= - dependencies: - minimatch "0.3.0" - recursive-readdir@2.2.1: version "2.2.1" resolved "https://registry.yarnpkg.com/recursive-readdir/-/recursive-readdir-2.2.1.tgz#90ef231d0778c5ce093c9a48d74e5c5422d13a99" @@ -16283,6 +16263,13 @@ recursive-readdir@2.2.1: dependencies: minimatch "3.0.3" +recursive-readdir@^2.2.2: + version "2.2.2" + resolved "https://registry.yarnpkg.com/recursive-readdir/-/recursive-readdir-2.2.2.tgz#9946fb3274e1628de6e36b2f6714953b4845094f" + integrity sha512-nRCcW9Sj7NuZwa2XvH9co8NPeXUBhZP7CRKJtU+cS6PW9FpCIFoI5ib0NT1ZrbNuPoRy0ylyCaUL8Gih4LSyFg== + dependencies: + minimatch "3.0.4" + redent@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/redent/-/redent-1.0.0.tgz#cf916ab1fd5f1f16dfb20822dd6ec7f730c2afde" @@ -17426,11 +17413,6 @@ sift@^6.0.0: resolved "https://registry.yarnpkg.com/sift/-/sift-6.0.0.tgz#f93a778e5cbf05a5024ebc391e6b32511a6d1f82" integrity sha1-+Tp3jly/BaUCTrw5HmsyURptH4I= -sigmund@~1.0.0: - version "1.0.1" - resolved "https://registry.yarnpkg.com/sigmund/-/sigmund-1.0.1.tgz#3ff21f198cad2175f9f3b781853fd94d0d19b590" - integrity sha1-P/IfGYytIXX587eBhT/ZTQ0ZtZA= - signal-exit@^3.0.0, signal-exit@^3.0.2: version "3.0.2" resolved "https://registry.yarnpkg.com/signal-exit/-/signal-exit-3.0.2.tgz#b5fdc08f1287ea1178628e415e25132b73646c6d" From fde992e8946414c4fbdb5508f6242fde9d568f8c Mon Sep 17 00:00:00 2001 From: phacks Date: Sat, 8 Dec 2018 19:18:20 +0100 Subject: [PATCH 2/3] Rethrow errors without specific handling --- .../src/__tests__/gatsby-node.js | 12 ------------ packages/gatsby-remark-code-repls/src/gatsby-node.js | 6 ++---- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js b/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js index f55e419db3b0d..bdb2d9b605242 100644 --- a/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js +++ b/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js @@ -135,18 +135,6 @@ describe(`gatsby-remark-code-repls`, () => { } }) - it(`should wrap any non-error rejection from recursive-readdir in an Error instance`, async () => { - const rejectionMessage = `an error message` - - readdir.mockRejectedValue(rejectionMessage) - - try { - await createPages(createPagesParams) - } catch (err) { - expect(err).toEqual(Error(rejectionMessage)) - } - }) - it(`should not load any external packages by default`, async () => { readdir.mockResolvedValue([`file.js`]) diff --git a/packages/gatsby-remark-code-repls/src/gatsby-node.js b/packages/gatsby-remark-code-repls/src/gatsby-node.js index ebada4a0c7ea3..28775b34f9f60 100644 --- a/packages/gatsby-remark-code-repls/src/gatsby-node.js +++ b/packages/gatsby-remark-code-repls/src/gatsby-node.js @@ -75,9 +75,7 @@ exports.createPages = async ( } }) } catch (error) { - if (error instanceof Error) { - throw error - } - throw Error(error) + // retrow errors upstream + throw error } } From cb3902c223f66e52ceea34471701bc3777c319fd Mon Sep 17 00:00:00 2001 From: phacks Date: Mon, 4 Mar 2019 21:24:34 +0100 Subject: [PATCH 3/3] Better error handling with `reporter.panic` --- .../src/__tests__/gatsby-node.js | 8 ++++++++ .../gatsby-remark-code-repls/src/gatsby-node.js | 15 ++++++++++----- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js b/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js index bdb2d9b605242..be21fb86b260e 100644 --- a/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js +++ b/packages/gatsby-remark-code-repls/src/__tests__/gatsby-node.js @@ -5,10 +5,17 @@ jest.mock(`fs`, () => { } }) jest.mock(`recursive-readdir`, () => jest.fn()) +jest.mock(`gatsby-cli/lib/reporter`, () => { + return { + panic: jest.fn(), + } +}) const fs = require(`fs`) const readdir = require(`recursive-readdir`) +const reporter = require(`gatsby-cli/lib/reporter`) + const { OPTION_DEFAULT_HTML, OPTION_DEFAULT_REDIRECT_TEMPLATE_PATH, @@ -20,6 +27,7 @@ const createPagesParams = { actions: { createPage, }, + reporter, } describe(`gatsby-remark-code-repls`, () => { diff --git a/packages/gatsby-remark-code-repls/src/gatsby-node.js b/packages/gatsby-remark-code-repls/src/gatsby-node.js index 28775b34f9f60..a18e02fb333a7 100644 --- a/packages/gatsby-remark-code-repls/src/gatsby-node.js +++ b/packages/gatsby-remark-code-repls/src/gatsby-node.js @@ -12,7 +12,7 @@ const { } = require(`./constants`) exports.createPages = async ( - { actions }, + { actions, reporter }, { directory = OPTION_DEFAULT_LINK_TEXT, externals = [], @@ -27,11 +27,11 @@ exports.createPages = async ( const { createPage } = actions if (!fs.existsSync(directory)) { - throw Error(`Invalid REPL directory specified: "${directory}"`) + reporter.panic(`Invalid REPL directory specified: "${directory}"`) } if (!fs.existsSync(redirectTemplate)) { - throw Error( + reporter.panic( `Invalid REPL redirectTemplate specified: "${redirectTemplate}"` ) } @@ -75,7 +75,12 @@ exports.createPages = async ( } }) } catch (error) { - // retrow errors upstream - throw error + reporter.panic( + ` + Error in gatsby-remark-code-repls plugin: cannot read directory ${directory}. + More details can be found in the error reporting below. + `, + error + ) } }