From dd5415f626537d3900bb1c16fa815b96cdccde83 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Tue, 16 May 2017 22:26:05 -0500 Subject: [PATCH 1/6] Add module scope plugin --- packages/react-dev-utils/ModuleScopePlugin.js | 62 +++++++++++++++++++ .../config/webpack.config.dev.js | 2 + .../config/webpack.config.prod.js | 2 + 3 files changed, 66 insertions(+) create mode 100644 packages/react-dev-utils/ModuleScopePlugin.js diff --git a/packages/react-dev-utils/ModuleScopePlugin.js b/packages/react-dev-utils/ModuleScopePlugin.js new file mode 100644 index 00000000000..3fb2b4261a7 --- /dev/null +++ b/packages/react-dev-utils/ModuleScopePlugin.js @@ -0,0 +1,62 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +'use strict'; + +const path = require('path'); + +class ModuleScopePlugin { + constructor(appSrc) { + this.appSrc = appSrc; + } + + apply(resolver) { + const { appSrc } = this; + resolver.plugin('file', (request, callback) => { + // Unknown issuer, probably webpack internals + if (!request.context.issuer) { + return callback(); + } + if ( + // If this resolves to a node_module, we don't care what happens next + request.descriptionFileRoot.indexOf('/node_modules/') !== -1 || + // Make sure this request was manual + !request.__innerRequest_request + ) { + return callback(); + } + // Resolve the issuer from our appSrc and make sure it's one of our files + // Maybe an indexOf === 0 would be better? + const relative = path.relative(appSrc, request.context.issuer); + // If we go back, not our request! + if (relative[0] === '.') { + return callback(); + } + const requestRelative = path.relative( + appSrc, + path.resolve( + path.dirname(request.context.issuer), + request.__innerRequest_request + ) + ); + if (requestRelative[0] === '.') { + callback( + new Error( + `You attempted to require ${request.__innerRequest_request}, which falls outside of src/.` + ), + request + ); + } else { + callback(); + } + }); + } +} + +module.exports = ModuleScopePlugin; diff --git a/packages/react-scripts/config/webpack.config.dev.js b/packages/react-scripts/config/webpack.config.dev.js index f90ca6bd09e..2ebaaf119ed 100644 --- a/packages/react-scripts/config/webpack.config.dev.js +++ b/packages/react-scripts/config/webpack.config.dev.js @@ -18,6 +18,7 @@ const CaseSensitivePathsPlugin = require('case-sensitive-paths-webpack-plugin'); const InterpolateHtmlPlugin = require('react-dev-utils/InterpolateHtmlPlugin'); const WatchMissingNodeModulesPlugin = require('react-dev-utils/WatchMissingNodeModulesPlugin'); const eslintFormatter = require('react-dev-utils/eslintFormatter'); +const ModuleScopePlugin = require('react-dev-utils/ModuleScopePlugin'); const getClientEnvironment = require('./env'); const paths = require('./paths'); @@ -106,6 +107,7 @@ module.exports = { // https://www.smashingmagazine.com/2016/08/a-glimpse-into-the-future-with-react-native-for-web/ 'react-native': 'react-native-web', }, + plugins: [new ModuleScopePlugin(paths.appSrc)], }, module: { strictExportPresence: true, diff --git a/packages/react-scripts/config/webpack.config.prod.js b/packages/react-scripts/config/webpack.config.prod.js index 182105c756a..6e9e0484b87 100644 --- a/packages/react-scripts/config/webpack.config.prod.js +++ b/packages/react-scripts/config/webpack.config.prod.js @@ -18,6 +18,7 @@ const ExtractTextPlugin = require('extract-text-webpack-plugin'); const ManifestPlugin = require('webpack-manifest-plugin'); const InterpolateHtmlPlugin = require('react-dev-utils/InterpolateHtmlPlugin'); const eslintFormatter = require('react-dev-utils/eslintFormatter'); +const ModuleScopePlugin = require('react-dev-utils/ModuleScopePlugin'); const paths = require('./paths'); const getClientEnvironment = require('./env'); @@ -102,6 +103,7 @@ module.exports = { // Support React Native Web // https://www.smashingmagazine.com/2016/08/a-glimpse-into-the-future-with-react-native-for-web/ 'react-native': 'react-native-web', + plugins: [new ModuleScopePlugin(paths.appSrc)], }, }, module: { From 871bf4ef82a186c7384d58af89fec5592f15e182 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Wed, 17 May 2017 00:33:30 -0500 Subject: [PATCH 2/6] Oops --- packages/react-dev-utils/package.json | 1 + packages/react-scripts/config/webpack.config.prod.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react-dev-utils/package.json b/packages/react-dev-utils/package.json index d52fe30fd86..0a22499ca01 100644 --- a/packages/react-dev-utils/package.json +++ b/packages/react-dev-utils/package.json @@ -21,6 +21,7 @@ "getProcessForPort.js", "InterpolateHtmlPlugin.js", "launchEditor.js", + "ModuleScopePlugin.js", "openBrowser.js", "openChrome.applescript", "prepareProxy.js", diff --git a/packages/react-scripts/config/webpack.config.prod.js b/packages/react-scripts/config/webpack.config.prod.js index 6e9e0484b87..d853ec24860 100644 --- a/packages/react-scripts/config/webpack.config.prod.js +++ b/packages/react-scripts/config/webpack.config.prod.js @@ -103,8 +103,8 @@ module.exports = { // Support React Native Web // https://www.smashingmagazine.com/2016/08/a-glimpse-into-the-future-with-react-native-for-web/ 'react-native': 'react-native-web', - plugins: [new ModuleScopePlugin(paths.appSrc)], }, + plugins: [new ModuleScopePlugin(paths.appSrc)], }, module: { strictExportPresence: true, From 3ee431db055f8f9d51bf56f313c01129c14aedf4 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Wed, 17 May 2017 00:39:15 -0500 Subject: [PATCH 3/6] Add comments --- packages/react-dev-utils/ModuleScopePlugin.js | 4 +++- packages/react-scripts/config/webpack.config.dev.js | 9 ++++++++- packages/react-scripts/config/webpack.config.prod.js | 9 ++++++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/react-dev-utils/ModuleScopePlugin.js b/packages/react-dev-utils/ModuleScopePlugin.js index 3fb2b4261a7..2e634f9ab5d 100644 --- a/packages/react-dev-utils/ModuleScopePlugin.js +++ b/packages/react-dev-utils/ModuleScopePlugin.js @@ -34,10 +34,11 @@ class ModuleScopePlugin { // Resolve the issuer from our appSrc and make sure it's one of our files // Maybe an indexOf === 0 would be better? const relative = path.relative(appSrc, request.context.issuer); - // If we go back, not our request! + // If it's not in src/ or a subdirectory, not our request! if (relative[0] === '.') { return callback(); } + // Find path from src to the requested file const requestRelative = path.relative( appSrc, path.resolve( @@ -45,6 +46,7 @@ class ModuleScopePlugin { request.__innerRequest_request ) ); + // Error if in a parent directory of src/ if (requestRelative[0] === '.') { callback( new Error( diff --git a/packages/react-scripts/config/webpack.config.dev.js b/packages/react-scripts/config/webpack.config.dev.js index 2ebaaf119ed..3d198d28a34 100644 --- a/packages/react-scripts/config/webpack.config.dev.js +++ b/packages/react-scripts/config/webpack.config.dev.js @@ -107,7 +107,14 @@ module.exports = { // https://www.smashingmagazine.com/2016/08/a-glimpse-into-the-future-with-react-native-for-web/ 'react-native': 'react-native-web', }, - plugins: [new ModuleScopePlugin(paths.appSrc)], + plugins: [ + // Prevents users from importing files from outside of src/ (or node_modules/). + // This often causes confusion because we only process files within src/ with babel. + // To fix this, we prevent you from importing files out of src/ -- if you'd like to, + // please link the files into your node_modules/ and let module-resolution kick in. + // Make sure your source files are compiled, as they will not be processed in any way. + new ModuleScopePlugin(paths.appSrc), + ], }, module: { strictExportPresence: true, diff --git a/packages/react-scripts/config/webpack.config.prod.js b/packages/react-scripts/config/webpack.config.prod.js index d853ec24860..9742e7ff50a 100644 --- a/packages/react-scripts/config/webpack.config.prod.js +++ b/packages/react-scripts/config/webpack.config.prod.js @@ -104,7 +104,14 @@ module.exports = { // https://www.smashingmagazine.com/2016/08/a-glimpse-into-the-future-with-react-native-for-web/ 'react-native': 'react-native-web', }, - plugins: [new ModuleScopePlugin(paths.appSrc)], + plugins: [ + // Prevents users from importing files from outside of src/ (or node_modules/). + // This often causes confusion because we only process files within src/ with babel. + // To fix this, we prevent you from importing files out of src/ -- if you'd like to, + // please link the files into your node_modules/ and let module-resolution kick in. + // Make sure your source files are compiled, as they will not be processed in any way. + new ModuleScopePlugin(paths.appSrc), + ], }, module: { strictExportPresence: true, From a955263125d9b2ae995f6a45c77ead15ef59b7c0 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Wed, 17 May 2017 01:35:12 -0500 Subject: [PATCH 4/6] Check windows seps too --- packages/react-dev-utils/ModuleScopePlugin.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-dev-utils/ModuleScopePlugin.js b/packages/react-dev-utils/ModuleScopePlugin.js index 2e634f9ab5d..168823b4355 100644 --- a/packages/react-dev-utils/ModuleScopePlugin.js +++ b/packages/react-dev-utils/ModuleScopePlugin.js @@ -26,6 +26,7 @@ class ModuleScopePlugin { if ( // If this resolves to a node_module, we don't care what happens next request.descriptionFileRoot.indexOf('/node_modules/') !== -1 || + request.descriptionFileRoot.indexOf('\\node_modules\\') !== -1 || // Make sure this request was manual !request.__innerRequest_request ) { From 322681cdb274c8882cda532d23abab96a14f224c Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 17 May 2017 12:14:25 +0100 Subject: [PATCH 5/6] More descriptive error --- packages/react-dev-utils/ModuleScopePlugin.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/react-dev-utils/ModuleScopePlugin.js b/packages/react-dev-utils/ModuleScopePlugin.js index 168823b4355..fd70a2f408a 100644 --- a/packages/react-dev-utils/ModuleScopePlugin.js +++ b/packages/react-dev-utils/ModuleScopePlugin.js @@ -9,6 +9,7 @@ 'use strict'; +const chalk = require('chalk'); const path = require('path'); class ModuleScopePlugin { @@ -51,7 +52,9 @@ class ModuleScopePlugin { if (requestRelative[0] === '.') { callback( new Error( - `You attempted to require ${request.__innerRequest_request}, which falls outside of src/.` + `You attempted to import ${chalk.cyan(request.__innerRequest_request)} which falls outside of the project ${chalk.cyan('src/')} directory. ` + + `Relative imports outside of ${chalk.cyan('src/')} are not supported. ` + + `You can either move it inside ${chalk.cyan('src/')}, or add a symlink to it from project's ${chalk.cyan('node_modules/')}.` ), request ); From addbb017968ce03ba4ea09089a1047b9f9dd122a Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 17 May 2017 12:19:25 +0100 Subject: [PATCH 6/6] Document it --- packages/react-dev-utils/README.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/react-dev-utils/README.md b/packages/react-dev-utils/README.md index ef720449356..4a7effe217a 100644 --- a/packages/react-dev-utils/README.md +++ b/packages/react-dev-utils/README.md @@ -56,6 +56,26 @@ module.exports = { } ``` + +#### `new ModuleScopePlugin(appSrc: string)` + +This Webpack plugin ensures that relative imports from app's source directory don't reach outside of it. + +```js +var path = require('path'); +var ModuleScopePlugin = require('react-dev-utils/ModuleScopePlugin'); + + +module.exports = { + // ... + plugins: [ + new ModuleScopePlugin(paths.appSrc), + // ... + ], + // ... +} +``` + #### `new WatchMissingNodeModulesPlugin(nodeModulesPath: string)` This Webpack plugin ensures `npm install ` forces a project rebuild.