From 929f658b514a21c5ebe79d64ca6b1b0995c36c45 Mon Sep 17 00:00:00 2001 From: Edward Yong Gyun Kim Date: Tue, 12 Mar 2019 00:27:04 -0700 Subject: [PATCH 1/3] Handle path prefix on fromPath in createRedirect Related #12497 --- .../__tests__/__snapshots__/redirects.js.snap | 313 ++++++++++++++++++ .../gatsby/src/redux/__tests__/redirects.js | 139 ++++++++ packages/gatsby/src/redux/actions.js | 15 +- 3 files changed, 463 insertions(+), 4 deletions(-) create mode 100644 packages/gatsby/src/redux/__tests__/__snapshots__/redirects.js.snap create mode 100644 packages/gatsby/src/redux/__tests__/redirects.js diff --git a/packages/gatsby/src/redux/__tests__/__snapshots__/redirects.js.snap b/packages/gatsby/src/redux/__tests__/__snapshots__/redirects.js.snap new file mode 100644 index 0000000000000..ab045bba78589 --- /dev/null +++ b/packages/gatsby/src/redux/__tests__/__snapshots__/redirects.js.snap @@ -0,0 +1,313 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Add redirects allows you to add redirects 1`] = ` +Object { + "payload": Object { + "fromPath": "/old/hello-world", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "/new/hello-world", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects create redirects as permanent 1`] = ` +Object { + "payload": Object { + "fromPath": "/old/hello-world", + "isPermanent": true, + "redirectInBrowser": false, + "toPath": "/new/hello-world", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects creates redirects from the URL starts with // 1`] = ` +Object { + "payload": Object { + "fromPath": "//example.com", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "/new/hello-world-2", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects creates redirects from the URL starts with ftp 1`] = ` +Object { + "payload": Object { + "fromPath": "ftp://example.com", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "/new/hello-world-3", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects creates redirects from the URL starts with http 1`] = ` +Object { + "payload": Object { + "fromPath": "http://example.com", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "/new/hello-world-1", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects creates redirects from the URL starts with https 1`] = ` +Object { + "payload": Object { + "fromPath": "https://example.com", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "/new/hello-world-0", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects creates redirects from the URL starts with mailto 1`] = ` +Object { + "payload": Object { + "fromPath": "mailto:example@email.com", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "/new/hello-world-4", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects creates redirects to the URL starts with // 1`] = ` +Object { + "payload": Object { + "fromPath": "/old/hello-world-2", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "//example.com", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects creates redirects to the URL starts with ftp 1`] = ` +Object { + "payload": Object { + "fromPath": "/old/hello-world-3", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "ftp://example.com", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects creates redirects to the URL starts with http 1`] = ` +Object { + "payload": Object { + "fromPath": "/old/hello-world-1", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "http://example.com", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects creates redirects to the URL starts with https 1`] = ` +Object { + "payload": Object { + "fromPath": "/old/hello-world-0", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "https://example.com", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects creates redirects to the URL starts with mailto 1`] = ` +Object { + "payload": Object { + "fromPath": "/old/hello-world-4", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "mailto:example@email.com", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects creates redirects with in-browser redirect option 1`] = ` +Object { + "payload": Object { + "fromPath": "/old/hello-world", + "isPermanent": false, + "redirectInBrowser": true, + "toPath": "/new/hello-world", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects with path prefixs allows you to add redirects 1`] = ` +Object { + "payload": Object { + "fromPath": "/blog/old/hello-world", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "/blog/new/hello-world", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects with path prefixs create redirects as permanent 1`] = ` +Object { + "payload": Object { + "fromPath": "/blog/old/hello-world", + "isPermanent": true, + "redirectInBrowser": false, + "toPath": "/blog/new/hello-world", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects with path prefixs creates redirects from the URL starts with // 1`] = ` +Object { + "payload": Object { + "fromPath": "//example.com", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "/blog/new/hello-world-2", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects with path prefixs creates redirects from the URL starts with ftp 1`] = ` +Object { + "payload": Object { + "fromPath": "ftp://example.com", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "/blog/new/hello-world-3", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects with path prefixs creates redirects from the URL starts with http 1`] = ` +Object { + "payload": Object { + "fromPath": "http://example.com", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "/blog/new/hello-world-1", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects with path prefixs creates redirects from the URL starts with https 1`] = ` +Object { + "payload": Object { + "fromPath": "https://example.com", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "/blog/new/hello-world-0", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects with path prefixs creates redirects from the URL starts with mailto 1`] = ` +Object { + "payload": Object { + "fromPath": "mailto:example@email.com", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "/blog/new/hello-world-4", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects with path prefixs creates redirects to the URL starts with // 1`] = ` +Object { + "payload": Object { + "fromPath": "/blog/old/hello-world-2", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "//example.com", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects with path prefixs creates redirects to the URL starts with ftp 1`] = ` +Object { + "payload": Object { + "fromPath": "/blog/old/hello-world-3", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "ftp://example.com", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects with path prefixs creates redirects to the URL starts with http 1`] = ` +Object { + "payload": Object { + "fromPath": "/blog/old/hello-world-1", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "http://example.com", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects with path prefixs creates redirects to the URL starts with https 1`] = ` +Object { + "payload": Object { + "fromPath": "/blog/old/hello-world-0", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "https://example.com", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects with path prefixs creates redirects to the URL starts with mailto 1`] = ` +Object { + "payload": Object { + "fromPath": "/blog/old/hello-world-4", + "isPermanent": false, + "redirectInBrowser": false, + "toPath": "mailto:example@email.com", + }, + "type": "CREATE_REDIRECT", +} +`; + +exports[`Add redirects with path prefixs creates redirects with in-browser redirect option 1`] = ` +Object { + "payload": Object { + "fromPath": "/blog/old/hello-world", + "isPermanent": false, + "redirectInBrowser": true, + "toPath": "/blog/new/hello-world", + }, + "type": "CREATE_REDIRECT", +} +`; diff --git a/packages/gatsby/src/redux/__tests__/redirects.js b/packages/gatsby/src/redux/__tests__/redirects.js new file mode 100644 index 0000000000000..a07af6a00ecdb --- /dev/null +++ b/packages/gatsby/src/redux/__tests__/redirects.js @@ -0,0 +1,139 @@ +"use strict" + +const { actions } = require(`../actions`) +const { store } = require(`../index`) + +jest.mock(`../index`, () => { + return { + store: { + getState: jest.fn(), + }, + dispath: () => {}, + } +}) + +const protocolArr = [ + [`https`, `https://example.com`], + [`http`, `http://example.com`], + [`//`, `//example.com`], + [`ftp`, `ftp://example.com`], + [`mailto`, `mailto:example@email.com`], +] + +describe(`Add redirects`, () => { + beforeEach(() => { + store.getState.mockReturnValue({ program: { pathPrefixs: false } }) + }) + + it(`allows you to add redirects`, () => { + const action = actions.createRedirect({ + fromPath: `/old/hello-world`, + toPath: `/new/hello-world`, + }) + + expect(action).toMatchSnapshot() + }) + it(`create redirects as permanent`, () => { + const action = actions.createRedirect({ + fromPath: `/old/hello-world`, + toPath: `/new/hello-world`, + isPermanent: true, + }) + + expect(action).toMatchSnapshot() + }) + + it(`creates redirects with in-browser redirect option`, () => { + const action = actions.createRedirect({ + fromPath: `/old/hello-world`, + toPath: `/new/hello-world`, + redirectInBrowser: true, + }) + + expect(action).toMatchSnapshot() + }) + + protocolArr.forEach(([protocol, toPath], index) => { + it(`creates redirects to the URL starts with ${protocol}`, () => { + const action = actions.createRedirect({ + fromPath: `/old/hello-world-${index}`, + toPath, + }) + + expect(action).toMatchSnapshot() + }) + }) + + protocolArr.forEach(([protocol, fromPath], index) => { + it(`creates redirects from the URL starts with ${protocol}`, () => { + const action = actions.createRedirect({ + fromPath, + toPath: `/new/hello-world-${index}`, + }) + + expect(action).toMatchSnapshot() + }) + }) +}) + +describe(`Add redirects with path prefixs`, () => { + beforeEach(() => { + store.getState.mockReturnValue({ + program: { + prefixPaths: true, + }, + config: { + pathPrefix: `/blog`, + }, + }) + }) + it(`allows you to add redirects`, () => { + const action = actions.createRedirect({ + fromPath: `/old/hello-world`, + toPath: `/new/hello-world`, + }) + + expect(action).toMatchSnapshot() + }) + it(`create redirects as permanent`, () => { + const action = actions.createRedirect({ + fromPath: `/old/hello-world`, + toPath: `/new/hello-world`, + isPermanent: true, + }) + + expect(action).toMatchSnapshot() + }) + + it(`creates redirects with in-browser redirect option`, () => { + const action = actions.createRedirect({ + fromPath: `/old/hello-world`, + toPath: `/new/hello-world`, + redirectInBrowser: true, + }) + + expect(action).toMatchSnapshot() + }) + + protocolArr.forEach(([protocol, toPath], index) => { + it(`creates redirects to the URL starts with ${protocol}`, () => { + const action = actions.createRedirect({ + fromPath: `/old/hello-world-${index}`, + toPath, + }) + + expect(action).toMatchSnapshot() + }) + }) + + protocolArr.forEach(([protocol, fromPath], index) => { + it(`creates redirects from the URL starts with ${protocol}`, () => { + const action = actions.createRedirect({ + fromPath, + toPath: `/new/hello-world-${index}`, + }) + + expect(action).toMatchSnapshot() + }) + }) +}) diff --git a/packages/gatsby/src/redux/actions.js b/packages/gatsby/src/redux/actions.js index 313a47c9e8677..bdd94e6ef5b24 100644 --- a/packages/gatsby/src/redux/actions.js +++ b/packages/gatsby/src/redux/actions.js @@ -1134,15 +1134,22 @@ actions.createRedirect = ({ // Parse urls to get their protocols // url.parse will not cover protocol-relative urls so do a separate check for those - const parsed = url.parse(toPath) - const isRelativeProtocol = toPath.startsWith(`//`) + const parsedFromPath = url.parse(fromPath) + const isFromPathRelativeProtocol = fromPath.startsWith(`//`) + const fromPathPrefix = + parsedFromPath.protocol != null || isFromPathRelativeProtocol + ? `` + : pathPrefix + + const parsedToPath = url.parse(toPath) + const isToPathRelativeProtocol = toPath.startsWith(`//`) const toPathPrefix = - parsed.protocol != null || isRelativeProtocol ? `` : pathPrefix + parsedToPath.protocol != null || isToPathRelativeProtocol ? `` : pathPrefix return { type: `CREATE_REDIRECT`, payload: { - fromPath: `${pathPrefix}${fromPath}`, + fromPath: `${fromPathPrefix}${fromPath}`, isPermanent, redirectInBrowser, toPath: `${toPathPrefix}${toPath}`, From 8408cb6733461ffb3be89dc3895d4e82539f51a7 Mon Sep 17 00:00:00 2001 From: Edward Yong Gyun Kim Date: Tue, 12 Mar 2019 11:23:11 -0700 Subject: [PATCH 2/3] Remove unnecessary use strict --- packages/gatsby/src/redux/__tests__/redirects.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/gatsby/src/redux/__tests__/redirects.js b/packages/gatsby/src/redux/__tests__/redirects.js index a07af6a00ecdb..e5a9707b3f633 100644 --- a/packages/gatsby/src/redux/__tests__/redirects.js +++ b/packages/gatsby/src/redux/__tests__/redirects.js @@ -1,5 +1,3 @@ -"use strict" - const { actions } = require(`../actions`) const { store } = require(`../index`) From 3e4fd2377789ef5bf3983ed890bda9e9e31d3aa4 Mon Sep 17 00:00:00 2001 From: Edward Yong Gyun Kim Date: Tue, 12 Mar 2019 11:25:04 -0700 Subject: [PATCH 3/3] Refactor createRedirect Remove duplicated logic and make it as maybeAddPathPrefix. --- packages/gatsby/src/redux/actions.js | 29 +++++++++++++--------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/packages/gatsby/src/redux/actions.js b/packages/gatsby/src/redux/actions.js index bdd94e6ef5b24..618e5520dec93 100644 --- a/packages/gatsby/src/redux/actions.js +++ b/packages/gatsby/src/redux/actions.js @@ -1101,6 +1101,17 @@ actions.setPluginStatus = ( } } +/** + * Check if path is absolute and add pathPrefix in front if it's not + */ +const maybeAddPathPrefix = (path, pathPrefix) => { + const parsed = url.parse(path) + const isRelativeProtocol = path.startsWith(`//`) + return `${ + parsed.protocol != null || isRelativeProtocol ? `` : pathPrefix + }${path}` +} + /** * Create a redirect from one page to another. Server redirects don't work out * of the box. You must have a plugin setup to integrate the redirect data with @@ -1132,27 +1143,13 @@ actions.createRedirect = ({ pathPrefix = store.getState().config.pathPrefix } - // Parse urls to get their protocols - // url.parse will not cover protocol-relative urls so do a separate check for those - const parsedFromPath = url.parse(fromPath) - const isFromPathRelativeProtocol = fromPath.startsWith(`//`) - const fromPathPrefix = - parsedFromPath.protocol != null || isFromPathRelativeProtocol - ? `` - : pathPrefix - - const parsedToPath = url.parse(toPath) - const isToPathRelativeProtocol = toPath.startsWith(`//`) - const toPathPrefix = - parsedToPath.protocol != null || isToPathRelativeProtocol ? `` : pathPrefix - return { type: `CREATE_REDIRECT`, payload: { - fromPath: `${fromPathPrefix}${fromPath}`, + fromPath: maybeAddPathPrefix(fromPath, pathPrefix), isPermanent, redirectInBrowser, - toPath: `${toPathPrefix}${toPath}`, + toPath: maybeAddPathPrefix(toPath, pathPrefix), ...rest, }, }