From ce17e2568e65a023deafcef1d023c592159e4a71 Mon Sep 17 00:00:00 2001 From: Blaine Kasten Date: Wed, 8 Jul 2020 08:42:09 -0500 Subject: [PATCH 1/4] fix(gatsby-link): Do not crash in unit tests when globals are undefined --- packages/gatsby-link/src/__tests__/index.js | 4 ++-- packages/gatsby-link/src/index.js | 13 ++++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/gatsby-link/src/__tests__/index.js b/packages/gatsby-link/src/__tests__/index.js index 82eaefb5c9dc9..c2a4b62956937 100644 --- a/packages/gatsby-link/src/__tests__/index.js +++ b/packages/gatsby-link/src/__tests__/index.js @@ -94,8 +94,8 @@ describe(``, () => { }) it(`does not fail with missing __BASE_PATH__`, () => { - global.__PATH_PREFIX__ = `` - global.__BASE_PATH__ = undefined + delete global.__PATH_PREFIX__ + delete global.__BASE_PATH__ const source = createMemorySource(`/active`) diff --git a/packages/gatsby-link/src/index.js b/packages/gatsby-link/src/index.js index 052a3d98a85eb..db66837631dd7 100644 --- a/packages/gatsby-link/src/index.js +++ b/packages/gatsby-link/src/index.js @@ -9,7 +9,7 @@ export { parsePath } const isAbsolutePath = path => path?.startsWith(`/`) -export function withPrefix(path, prefix = __BASE_PATH__) { +export function withPrefix(path, prefix = getGlobalBasePrefix()) { if (!isLocalLink(path)) { return path } @@ -17,13 +17,20 @@ export function withPrefix(path, prefix = __BASE_PATH__) { if (path.startsWith(`./`) || path.startsWith(`../`)) { return path } - const base = prefix ?? __PATH_PREFIX__ ?? `/` + const base = prefix ?? getGlobalPathPrefix() ?? `/` return `${base?.endsWith(`/`) ? base.slice(0, -1) : base}${ path.startsWith(`/`) ? path : `/${path}` }` } +// These global values are wrapped in typeof clauses to ensure the values exist. +// This is especially problematic in unit testing of this component. +const getGlobalPathPrefix = () => + typeof __PATH_PREFIX__ !== `undefined` ? __PATH_PREFIX__ : undefined +const getGlobalBasePrefix = () => + typeof __BASE_PATH__ !== `undefined` ? __BASE_PATH__ : undefined + const isLocalLink = path => path && !path.startsWith(`http://`) && @@ -31,7 +38,7 @@ const isLocalLink = path => !path.startsWith(`//`) export function withAssetPrefix(path) { - return withPrefix(path, __PATH_PREFIX__) + return withPrefix(path, getGlobalPathPrefix()) } function absolutify(path, current) { From 572e68a2c5123f4820dadcab8dc1c6336ea7d01b Mon Sep 17 00:00:00 2001 From: Blaine Kasten Date: Thu, 9 Jul 2020 09:03:57 -0500 Subject: [PATCH 2/4] wrap in production checks --- packages/gatsby-link/src/index.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/gatsby-link/src/index.js b/packages/gatsby-link/src/index.js index db66837631dd7..7206979c63e8d 100644 --- a/packages/gatsby-link/src/index.js +++ b/packages/gatsby-link/src/index.js @@ -27,9 +27,17 @@ export function withPrefix(path, prefix = getGlobalBasePrefix()) { // These global values are wrapped in typeof clauses to ensure the values exist. // This is especially problematic in unit testing of this component. const getGlobalPathPrefix = () => - typeof __PATH_PREFIX__ !== `undefined` ? __PATH_PREFIX__ : undefined + process.env.NODE_ENV !== `production` + ? typeof __PATH_PREFIX__ !== `undefined` + ? __PATH_PREFIX__ + : undefined + : __PATH_PREFIX__ const getGlobalBasePrefix = () => - typeof __BASE_PATH__ !== `undefined` ? __BASE_PATH__ : undefined + process.env.NODE_ENV !== `production` + ? typeof __BASE_PATH__ !== `undefined` + ? __BASE_PATH__ + : undefined + : __BASE_PATH__ const isLocalLink = path => path && From 91a894b13ccdf794575237aee0d7c733ad5f8414 Mon Sep 17 00:00:00 2001 From: Blaine Kasten Date: Thu, 9 Jul 2020 09:34:26 -0500 Subject: [PATCH 3/4] different approach --- packages/gatsby-link/src/index.js | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/packages/gatsby-link/src/index.js b/packages/gatsby-link/src/index.js index 7206979c63e8d..7da4a709f74ae 100644 --- a/packages/gatsby-link/src/index.js +++ b/packages/gatsby-link/src/index.js @@ -7,9 +7,18 @@ import { parsePath } from "./parse-path" export { parsePath } +if (process.env.NODE_ENV !== `production`) { + if (typeof __BASE_PATH__ === `undefined`) { + var __BASE_PATH__ = `` + } + if (typeof __PATH_PREFIX__ === `undefined`) { + var __PATH_PREFIX__ = `` + } +} + const isAbsolutePath = path => path?.startsWith(`/`) -export function withPrefix(path, prefix = getGlobalBasePrefix()) { +export function withPrefix(path, prefix = __BASE_PATH__) { if (!isLocalLink(path)) { return path } @@ -17,28 +26,13 @@ export function withPrefix(path, prefix = getGlobalBasePrefix()) { if (path.startsWith(`./`) || path.startsWith(`../`)) { return path } - const base = prefix ?? getGlobalPathPrefix() ?? `/` + const base = prefix ?? __PATH_PREFIX__ ?? `/` return `${base?.endsWith(`/`) ? base.slice(0, -1) : base}${ path.startsWith(`/`) ? path : `/${path}` }` } -// These global values are wrapped in typeof clauses to ensure the values exist. -// This is especially problematic in unit testing of this component. -const getGlobalPathPrefix = () => - process.env.NODE_ENV !== `production` - ? typeof __PATH_PREFIX__ !== `undefined` - ? __PATH_PREFIX__ - : undefined - : __PATH_PREFIX__ -const getGlobalBasePrefix = () => - process.env.NODE_ENV !== `production` - ? typeof __BASE_PATH__ !== `undefined` - ? __BASE_PATH__ - : undefined - : __BASE_PATH__ - const isLocalLink = path => path && !path.startsWith(`http://`) && @@ -46,7 +40,7 @@ const isLocalLink = path => !path.startsWith(`//`) export function withAssetPrefix(path) { - return withPrefix(path, getGlobalPathPrefix()) + return withPrefix(path, __PATH_PREFIX__) } function absolutify(path, current) { From b66bf5c6f49808e90fd1a2a80238eee461deb5c0 Mon Sep 17 00:00:00 2001 From: Blaine Kasten Date: Thu, 9 Jul 2020 10:01:52 -0500 Subject: [PATCH 4/4] Revert "different approach" This reverts commit 91a894b13ccdf794575237aee0d7c733ad5f8414. --- packages/gatsby-link/src/index.js | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/packages/gatsby-link/src/index.js b/packages/gatsby-link/src/index.js index 7da4a709f74ae..7206979c63e8d 100644 --- a/packages/gatsby-link/src/index.js +++ b/packages/gatsby-link/src/index.js @@ -7,18 +7,9 @@ import { parsePath } from "./parse-path" export { parsePath } -if (process.env.NODE_ENV !== `production`) { - if (typeof __BASE_PATH__ === `undefined`) { - var __BASE_PATH__ = `` - } - if (typeof __PATH_PREFIX__ === `undefined`) { - var __PATH_PREFIX__ = `` - } -} - const isAbsolutePath = path => path?.startsWith(`/`) -export function withPrefix(path, prefix = __BASE_PATH__) { +export function withPrefix(path, prefix = getGlobalBasePrefix()) { if (!isLocalLink(path)) { return path } @@ -26,13 +17,28 @@ export function withPrefix(path, prefix = __BASE_PATH__) { if (path.startsWith(`./`) || path.startsWith(`../`)) { return path } - const base = prefix ?? __PATH_PREFIX__ ?? `/` + const base = prefix ?? getGlobalPathPrefix() ?? `/` return `${base?.endsWith(`/`) ? base.slice(0, -1) : base}${ path.startsWith(`/`) ? path : `/${path}` }` } +// These global values are wrapped in typeof clauses to ensure the values exist. +// This is especially problematic in unit testing of this component. +const getGlobalPathPrefix = () => + process.env.NODE_ENV !== `production` + ? typeof __PATH_PREFIX__ !== `undefined` + ? __PATH_PREFIX__ + : undefined + : __PATH_PREFIX__ +const getGlobalBasePrefix = () => + process.env.NODE_ENV !== `production` + ? typeof __BASE_PATH__ !== `undefined` + ? __BASE_PATH__ + : undefined + : __BASE_PATH__ + const isLocalLink = path => path && !path.startsWith(`http://`) && @@ -40,7 +46,7 @@ const isLocalLink = path => !path.startsWith(`//`) export function withAssetPrefix(path) { - return withPrefix(path, __PATH_PREFIX__) + return withPrefix(path, getGlobalPathPrefix()) } function absolutify(path, current) {