fix(themes): Restrict the paths that count as shadowable for an issuer (

gatsbyjs#12930)

packages/gatsby/src/internal-plugins/webpack-theme-component-shadowing/__tests__/index.js

@@ -0,0 +1,109 @@
+import path from "path"
+import ShadowingPlugin from "../"
+
+describe(`Component Shadowing`, () => {
+  it(`gets matching themes`, () => {
+    const plugin = new ShadowingPlugin({
+      themes: [`a-theme`, `theme-b`, `gatsby-theme-c`].map(name => {
+        return {
+          themeName: name,
+          themeDir: path.join(path.sep, `some`, `place`, name),
+        }
+      }),
+    })
+    expect(
+      // simple request path to a theme's component
+      plugin.getMatchingThemesForPath(
+        path.join(
+          path.sep,
+          `some`,
+          `place`,
+          `a-theme`,
+          `src`,
+          `components`,
+          `a-component`
+        )
+      )
+    ).toEqual([`a-theme`])
+
+    expect(
+      // request to a shadowed component in theme b
+      // component-path is expected to be `a-theme/components/a-component`
+      plugin.getMatchingThemesForPath(
+        path.join(
+          path.sep,
+          `some`,
+          `place`,
+          `theme-b`,
+          `src`,
+          `a-theme`,
+          `components`,
+          `a-component`
+        )
+      )
+    ).toEqual([`theme-b`])
+  })
+
+  it(`can determine if the request path is in the shadow chain for the issuer`, () => {
+    const plugin = new ShadowingPlugin({
+      themes: [`a-theme`, `theme-b`, `gatsby-theme-c`].map(name => {
+        return {
+          themeName: name,
+          themeDir: path.join(path.sep, `some`, `place`, name),
+        }
+      }),
+    })
+    expect(
+      plugin.requestPathIsIssuerShadowPath({
+        // issuer is in `theme-b`
+        issuerPath: path.join(
+          path.sep,
+          `some`,
+          `place`,
+          `theme-b`,
+          `src`,
+          `a-theme`,
+          `components`,
+          `a-component`
+        ),
+        // require'ing a file it is a "shadow child" of in a-theme
+        requestPath: path.join(
+          path.sep,
+          `some`,
+          `place`,
+          `a-theme`,
+          `src`,
+          `components`,
+          `a-component`
+        ),
+      })
+    ).toEqual(true)
+
+    expect(
+      plugin.requestPathIsIssuerShadowPath({
+        // issuer is in `theme-b`
+        issuerPath: path.join(
+          path.sep,
+          `some`,
+          `place`,
+          `theme-b`,
+          `src`,
+          `a-theme`,
+          `components`,
+          `a-component`
+        ),
+        // require'ing a file it is NOT a "shadow child" of, also in theme-b
+        // the `component-path` here would be "components/a-component"
+        requestPath: path.join(
+          path.sep,
+          `some`,
+          `place`,
+          `theme-b`,
+          `src`,
+          `components`,
+          `a-component`
+        ),
+      })
+    ).toEqual(false)
+  })
+})

packages/gatsby/src/internal-plugins/webpack-theme-component-shadowing/gatsby-node.js

@@ -11,7 +11,7 @@ exports.onCreateWebpackConfig = (
       resolve: {
         plugins: [
           new GatsbyThemeComponentShadowingResolverPlugin({
-            themes: themes.themes.map(({ themeName }) => themeName),
+            themes: themes.themes,
             projectRoot: program.directory,
           }),
         ],

packages/gatsby/src/internal-plugins/webpack-theme-component-shadowing/index.js

@@ -3,25 +3,23 @@ const debug = require(`debug`)(`gatsby:component-shadowing`)
 const fs = require(`fs`)
 const _ = require(`lodash`)
 
+const pathWithoutExtension = fullPath => {
+  const parsed = path.parse(fullPath)
+  return path.join(parsed.dir, parsed.name)
+}
+
 module.exports = class GatsbyThemeComponentShadowingResolverPlugin {
   cache = {}
 
   constructor({ projectRoot, themes }) {
-    debug(`themes list`, themes)
+    debug(`themes list`, themes.map(({ themeName }) => themeName))
     this.themes = themes
     this.projectRoot = projectRoot
   }
 
   apply(resolver) {
     resolver.plugin(`relative`, (request, callback) => {
-      // find out which theme's src/components dir we're requiring from
-      const allMatchingThemes = this.themes.filter(name =>
-        request.path.includes(path.join(name, `src`))
-      )
-
-      // The same theme can be included twice in the themes list causing multiple
-      // matches. This case should only be counted as a single match for that theme.
-      const matchingThemes = _.uniq(allMatchingThemes)
+      const matchingThemes = this.getMatchingThemesForPath(request.path)
 
       // 0 matching themes happens a lot for paths we don't want to handle
       // > 1 matching theme means we have a path like
@@ -33,46 +31,54 @@ module.exports = class GatsbyThemeComponentShadowingResolverPlugin {
           )} for path ${request.path}`
         )
       }
+
       if (matchingThemes.length !== 1) {
         return callback()
       }
+
       // theme is the theme package from which we're requiring the relative component
       const [theme] = matchingThemes
       // get the location of the component relative to src/
       const [, component] = request.path.split(path.join(theme, `src`))
 
-      /**
-       * if someone adds
-       * ```
-       * modules: [path.resolve(__dirname, 'src'), 'node_modules'],
-       * ```
-       * to the webpack config, `issuer` is `null`, so we skip this check.
-       * note that it's probably a bad idea in general to set `modules`
-       * like this in a theme, but we also shouldn't artificially break
-       * people that do.
-       */
-      if (request.context.issuer) {
-        const issuerExtension = path.extname(request.context.issuer)
-
-        if (
-          request.context.issuer
-            .slice(0, -issuerExtension.length)
-            .endsWith(component)
-        ) {
-          return resolver.doResolve(
-            `describedRelative`,
-            request,
-            null,
-            {},
-            callback
-          )
-        }
+      if (
+        /**
+         * if someone adds
+         * ```
+         * modules: [path.resolve(__dirname, 'src'), 'node_modules'],
+         * ```
+         * to the webpack config, `issuer` is `null`, so we skip this check.
+         * note that it's probably a bad idea in general to set `modules`
+         * like this in a theme, but we also shouldn't artificially break
+         * people that do.
+         */
+        request.context.issuer &&
+        /**
+         * An issuer is the file making the require request. It can
+         * be in a user's site or a theme. If the issuer is requesting
+         * a path in the shadow chain that it participates in, then we
+         * will let the request through as normal. Otherwise, we
+         * engage the shadowing algorithm.
+         */
+        this.requestPathIsIssuerShadowPath({
+          requestPath: request.path,
+          issuerPath: request.context.issuer,
+        })
+      ) {
+        return resolver.doResolve(
+          `describedRelative`,
+          request,
+          null,
+          {},
+          callback
+        )
       }
+
+      // This is the shadowing algorithm.
       const builtComponentPath = this.resolveComponentPath({
         matchingTheme: theme,
         themes: this.themes,
         component,
-        projectRoot: this.projectRoot,
       })
 
       return resolver.doResolve(
@@ -86,24 +92,17 @@ module.exports = class GatsbyThemeComponentShadowingResolverPlugin {
   }
 
   // check the cache, the user's project, and finally the theme files
-  resolveComponentPath({
-    matchingTheme: theme,
-    themes: ogThemes,
-    component,
-    projectRoot,
-  }) {
+  resolveComponentPath({ matchingTheme: theme, themes: ogThemes, component }) {
     // don't include matching theme in possible shadowing paths
-    const themes = ogThemes.filter(t => t !== theme)
+    const themes = ogThemes.filter(({ themeName }) => themeName !== theme)
     if (!this.cache[`${theme}-${component}`]) {
       this.cache[`${theme}-${component}`] = [
         path.join(path.resolve(`.`), `src`, theme),
       ]
         .concat(
           Array.from(themes)
             .reverse()
-            .map(aTheme =>
-              path.join(path.dirname(require.resolve(aTheme)), `src`, theme)
-            )
+            .map(({ themeDir }) => path.join(themeDir, `src`, theme))
         )
         .map(dir => path.join(dir, component))
         .find(possibleComponentPath => {
@@ -134,4 +133,48 @@ module.exports = class GatsbyThemeComponentShadowingResolverPlugin {
 
     return this.cache[`${theme}-${component}`]
   }
+
+  getMatchingThemesForPath(filepath) {
+    // find out which theme's src/components dir we're requiring from
+    const allMatchingThemes = this.themes.filter(({ themeName }) =>
+      filepath.includes(path.join(themeName, `src`))
+    )
+
+    // The same theme can be included twice in the themes list causing multiple
+    // matches. This case should only be counted as a single match for that theme.
+    return _.uniq(allMatchingThemes.map(({ themeName }) => themeName))
+  }
+
+  // given a theme name, return all of the possible shadow locations
+  getBaseShadowDirsForThemes(theme) {
+    return Array.from(this.themes)
+      .reverse()
+      .map(({ themeName, themeDir }) => {
+        if (themeName === theme) {
+          return path.join(themeDir, `src`)
+        } else {
+          return path.join(themeDir, `src`, theme)
+        }
+      })
+  }
+
+  requestPathIsIssuerShadowPath({ requestPath, issuerPath }) {
+    // get the issuer's theme
+    const matchingThemes = this.getMatchingThemesForPath(requestPath)
+    if (matchingThemes.length !== 1) {
+      return false
+    }
+    const [theme] = matchingThemes
+
+    // get the location of the component relative to src/
+    const [, component] = requestPath.split(path.join(theme, `src`))
+
+    // get list of potential shadow locations
+    const shadowFiles = this.getBaseShadowDirsForThemes(theme).map(dir =>
+      path.join(dir, component)
+    )
+
+    // if the issuer is requesting a path that is a potential shadow path of itself
+    return shadowFiles.includes(pathWithoutExtension(issuerPath))
+  }
 }