From e040947bb3ee5995d540cc51f165938f7ec6eb8a Mon Sep 17 00:00:00 2001 From: Jason Lengstorf Date: Thu, 11 Apr 2019 18:34:09 -0700 Subject: [PATCH] fix(themes): Restrict the paths that count as shadowable for an issuer (#12930) --- .../__tests__/index.js | 109 ++++++++++++++ .../gatsby-node.js | 2 +- .../index.js | 135 ++++++++++++------ 3 files changed, 199 insertions(+), 47 deletions(-) create mode 100644 packages/gatsby/src/internal-plugins/webpack-theme-component-shadowing/__tests__/index.js diff --git a/packages/gatsby/src/internal-plugins/webpack-theme-component-shadowing/__tests__/index.js b/packages/gatsby/src/internal-plugins/webpack-theme-component-shadowing/__tests__/index.js new file mode 100644 index 0000000000000..2b90c58e392da --- /dev/null +++ b/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) + }) +}) diff --git a/packages/gatsby/src/internal-plugins/webpack-theme-component-shadowing/gatsby-node.js b/packages/gatsby/src/internal-plugins/webpack-theme-component-shadowing/gatsby-node.js index 1f19141886a1c..a4bf55e4c98ae 100644 --- a/packages/gatsby/src/internal-plugins/webpack-theme-component-shadowing/gatsby-node.js +++ b/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, }), ], diff --git a/packages/gatsby/src/internal-plugins/webpack-theme-component-shadowing/index.js b/packages/gatsby/src/internal-plugins/webpack-theme-component-shadowing/index.js index 0af131788065f..a2b39d1fc4e81 100644 --- a/packages/gatsby/src/internal-plugins/webpack-theme-component-shadowing/index.js +++ b/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,14 +92,9 @@ 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), @@ -101,9 +102,7 @@ module.exports = class GatsbyThemeComponentShadowingResolverPlugin { .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)) + } }