Fix gatsby-plugin-manifest using withAssetPrefix in manifest file lin…

…k, where it gets improperly sanitized (#26844) * Do not use withAssetPrefix on manifest href Since Gatsby core removes the `assetPrefix` from `<link>` elements' `href` if their `rel` is set to `manifest`, we shouldn't try to add it here. * use withPrefix in client-side manifest link localization * improved manifest plugin tests to consider assetPrefix correctly
gatsbyjs · Sep 18, 2020 · 8325026 · 8325026
1 parent 55052ef
commit 8325026
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 8 deletions.
diff --git a/packages/gatsby-plugin-manifest/src/__tests__/gatsby-ssr.js b/packages/gatsby-plugin-manifest/src/__tests__/gatsby-ssr.js
@@ -30,23 +30,46 @@ describe(`gatsby-plugin-manifest`, () => {
     headComponents = []
   })
 
-  it(`Creates href attributes using pathPrefix`, () => {
-    global.__PATH_PREFIX__ = `/path-prefix`
+  it(`Creates href attributes using full pathPrefix for non-webmanifest links`, () => {
+    global.__BASE_PATH__ = `/base-path`
+    global.__PATH_PREFIX__ = `http://path-prefix.com${global.__BASE_PATH__}`
 
     onRenderBody(ssrArgs, {
       icon: defaultIcon,
       theme_color: `#000000`,
     })
 
     headComponents
-      .filter(component => component.type === `link`)
+      .filter(
+        component =>
+          component.type === `link` && component.props.rel !== `manifest`
+      )
       .forEach(component => {
         expect(component.props.href).toEqual(
-          expect.stringMatching(/^\/path-prefix\//)
+          expect.stringMatching(new RegExp(`^${global.__PATH_PREFIX__}`))
         )
       })
   })
 
+  it(`Creates href attributes using pathPrefix without assetPrefix for the webmanifest link`, () => {
+    global.__BASE_PATH__ = `/base-path`
+    global.__PATH_PREFIX__ = `http://path-prefix.com${global.__BASE_PATH__}`
+
+    onRenderBody(ssrArgs, {
+      icon: defaultIcon,
+      theme_color: `#000000`,
+    })
+
+    const component = headComponents.find(
+      component =>
+        component.type === `link` && component.props.rel === `manifest`
+    )
+
+    expect(component.props.href).toEqual(
+      expect.stringMatching(new RegExp(`^${global.__BASE_PATH__}`))
+    )
+  })
+
   describe(`Manifest Link Generation`, () => {
     it(`Adds a "theme color" meta tag to head if "theme_color_in_head" is not provided`, () => {
       onRenderBody(ssrArgs, {

diff --git a/packages/gatsby-plugin-manifest/src/gatsby-browser.js b/packages/gatsby-plugin-manifest/src/gatsby-browser.js
@@ -1,11 +1,9 @@
 /* global __MANIFEST_PLUGIN_HAS_LOCALISATION__ */
-import { withPrefix as fallbackWithPrefix, withAssetPrefix } from "gatsby"
+import { withPrefix } from "gatsby"
 import getManifestForPathname from "./get-manifest-pathname"
 
 // when we don't have localisation in our manifest, we tree shake everything away
 if (__MANIFEST_PLUGIN_HAS_LOCALISATION__) {
-  const withPrefix = withAssetPrefix || fallbackWithPrefix
-
   exports.onRouteUpdate = function ({ location }, pluginOptions) {
     const { localize } = pluginOptions
     const manifestFilename = getManifestForPathname(location.pathname, localize)

diff --git a/packages/gatsby-plugin-manifest/src/gatsby-ssr.js b/packages/gatsby-plugin-manifest/src/gatsby-ssr.js
@@ -63,7 +63,7 @@ exports.onRenderBody = (
     <link
       key={`gatsby-plugin-manifest-link`}
       rel="manifest"
-      href={withPrefix(`/${manifestFileName}`)}
+      href={fallbackWithPrefix(`/${manifestFileName}`)}
       crossOrigin={crossOrigin}
     />
   )