From ea653be77026f18c326f891d7feea3ddcae3ca94 Mon Sep 17 00:00:00 2001 From: Dustin Schau Date: Mon, 12 Feb 2018 11:18:10 -0800 Subject: [PATCH 1/4] fix: prevent invalid graphql field names from being created Note: This will currently transform (for example) `/*` into `_` so I'm not quite sure what the best course of action is to replace that Fixes #3487; Fixes #2925 --- .../gatsby/src/schema/__tests__/create-key.js | 24 +++++++++++++++++++ packages/gatsby/src/schema/create-key.js | 11 +++++++-- yarn.lock | 2 +- 3 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 packages/gatsby/src/schema/__tests__/create-key.js diff --git a/packages/gatsby/src/schema/__tests__/create-key.js b/packages/gatsby/src/schema/__tests__/create-key.js new file mode 100644 index 0000000000000..e0f679b7a8414 --- /dev/null +++ b/packages/gatsby/src/schema/__tests__/create-key.js @@ -0,0 +1,24 @@ +const createKey = require(`../create-key`) + +describe(`createKey`, () => { + it(`leaves valid strings as is`, () => { + ;[ + [`01234`, `01234`], + [`description`, `description`], + [`_hello`, `_hello`], + ].forEach(([input, output]) => { + expect(createKey(input)).toBe(output) + }) + }) + + it(`replaces invalid characters`, () => { + ;[ + [`/hello`, `_hello`], + [`~/path/to/some/module`, `_path_to_some_module`], + [`/*`, `_`], + [`/*.js`, `_js`], + ].forEach(([input, output]) => { + expect(createKey(input)).toBe(output) + }) + }) +}) diff --git a/packages/gatsby/src/schema/create-key.js b/packages/gatsby/src/schema/create-key.js index 77d43f5e5c241..bc6b1bd3a57ca 100644 --- a/packages/gatsby/src/schema/create-key.js +++ b/packages/gatsby/src/schema/create-key.js @@ -1,6 +1,6 @@ // @flow const invariant = require(`invariant`) -const regex = new RegExp(`[^a-zA-Z0-9_]`, `g`) +const nonAlphaNumericExpr = new RegExp(`[^a-zA-Z0-9_]`, `g`) /** * GraphQL field names must be a string and cannot contain anything other than @@ -14,5 +14,12 @@ module.exports = (key: string): string => { `Graphql field name (key) is not a string -> ${key}` ) - return key.replace(regex, `_`) + const replaced = key.replace(nonAlphaNumericExpr, `_`) + + // TODO: figure out what to replace this with _OR_ change the expr + if (replaced.match(/^__/)) { + return replaced.replace(/^_{2,}/, `_`) + } + + return replaced } diff --git a/yarn.lock b/yarn.lock index 86044855e8398..fdf56f486a1c3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -549,7 +549,7 @@ axios@^0.17.1: follow-redirects "^1.2.5" is-buffer "^1.1.5" -"axios@github:contentful/axios#fix/https-via-http-proxy": +axios@contentful/axios#fix/https-via-http-proxy: version "0.17.1" resolved "https://codeload.github.com/contentful/axios/tar.gz/4b06f4a63db3ac16c99f7c61b584ef0e6d11f1af" dependencies: From 560d058fd8e17cbfc7af616af9ce0e1a35e9f5f4 Mon Sep 17 00:00:00 2001 From: Dustin Schau Date: Mon, 12 Feb 2018 11:21:23 -0800 Subject: [PATCH 2/4] chore: revert yarn lock change --- yarn.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn.lock b/yarn.lock index fdf56f486a1c3..86044855e8398 100644 --- a/yarn.lock +++ b/yarn.lock @@ -549,7 +549,7 @@ axios@^0.17.1: follow-redirects "^1.2.5" is-buffer "^1.1.5" -axios@contentful/axios#fix/https-via-http-proxy: +"axios@github:contentful/axios#fix/https-via-http-proxy": version "0.17.1" resolved "https://codeload.github.com/contentful/axios/tar.gz/4b06f4a63db3ac16c99f7c61b584ef0e6d11f1af" dependencies: From 563b27128e4335d21de159275f60f01c8503eca4 Mon Sep 17 00:00:00 2001 From: Dustin Schau Date: Mon, 12 Feb 2018 11:29:38 -0800 Subject: [PATCH 3/4] test: simplify tests --- packages/gatsby/src/schema/__tests__/create-key.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/gatsby/src/schema/__tests__/create-key.js b/packages/gatsby/src/schema/__tests__/create-key.js index e0f679b7a8414..eb4a28578422d 100644 --- a/packages/gatsby/src/schema/__tests__/create-key.js +++ b/packages/gatsby/src/schema/__tests__/create-key.js @@ -2,12 +2,8 @@ const createKey = require(`../create-key`) describe(`createKey`, () => { it(`leaves valid strings as is`, () => { - ;[ - [`01234`, `01234`], - [`description`, `description`], - [`_hello`, `_hello`], - ].forEach(([input, output]) => { - expect(createKey(input)).toBe(output) + ;[`01234`, `validstring`, `_hello`, `_`].forEach(input => { + expect(createKey(input)).toBe(input) }) }) From 383652163b5729859f9721edb0faf08fea297b9d Mon Sep 17 00:00:00 2001 From: Dustin Schau Date: Mon, 12 Feb 2018 13:01:32 -0800 Subject: [PATCH 4/4] fix: dasherize after leading underscore --- packages/gatsby/src/schema/__tests__/create-key.js | 12 +++++++++--- packages/gatsby/src/schema/create-key.js | 4 ++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/packages/gatsby/src/schema/__tests__/create-key.js b/packages/gatsby/src/schema/__tests__/create-key.js index eb4a28578422d..da6484dda1888 100644 --- a/packages/gatsby/src/schema/__tests__/create-key.js +++ b/packages/gatsby/src/schema/__tests__/create-key.js @@ -10,11 +10,17 @@ describe(`createKey`, () => { it(`replaces invalid characters`, () => { ;[ [`/hello`, `_hello`], - [`~/path/to/some/module`, `_path_to_some_module`], - [`/*`, `_`], - [`/*.js`, `_js`], + [`~/path/to/some/module`, `_-path-to-some-module`], + [`/*`, `_-`], + [`/*.js`, `_--js`], ].forEach(([input, output]) => { expect(createKey(input)).toBe(output) }) }) + + it(`does not generate same key for different input`, () => { + ;[[`/*.js`, `*js`]].forEach(([one, two]) => { + expect(createKey(one)).not.toBe(createKey(two)) + }) + }) }) diff --git a/packages/gatsby/src/schema/create-key.js b/packages/gatsby/src/schema/create-key.js index bc6b1bd3a57ca..4cb78499f5552 100644 --- a/packages/gatsby/src/schema/create-key.js +++ b/packages/gatsby/src/schema/create-key.js @@ -16,9 +16,9 @@ module.exports = (key: string): string => { const replaced = key.replace(nonAlphaNumericExpr, `_`) - // TODO: figure out what to replace this with _OR_ change the expr + // key is invalid; normalize with a leading underscore and dasherize rest if (replaced.match(/^__/)) { - return replaced.replace(/^_{2,}/, `_`) + return replaced.replace(/_/g, (char, index) => (index === 0 ? `_` : `-`)) } return replaced