fix NODE_PATH imports in create-react-app

[bugzpodder]

Fixes facebook/create-react-app#5643

[arcanis]

Thanks! Do you really need NODE_PATH, though? Can't you just add your src directory to the configuration, like this?:

{
  resolve: {
    modules: [
      `node_modules`,
      `${__dirname}/src`
    ]
  }
}

I'm a bit annoyed with adding NODE_PATH support since it is discouraged by the regular Node resolution, and PnP has no handling of it (because it would break the assumptions that it's been designed to enforce).

[bugzpodder]

I think this is what CRA does already see https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/config/webpack.config.prod.js
But the PNP plugin hijacks the the require and throws an error. I can test this again with a simple config to confirm. I don't like this PR as well, since it eats up a valid error. I think it should be scoped to current src if possible..

[bugzpodder]

I bootstrapped a simplier webpack repo: https://github.com/rwieruch/minimal-react-webpack-babel-setup

and it confirms that this plugin blocks resolve.modules from resolving properly.

diff --git a/webpack.config.js b/webpack.config.js
index 8eb9618..f956f55 100644
--- a/webpack.config.js
+++ b/webpack.config.js
@@ -1,3 +1,4 @@
+const PnpWebpackPlugin = require(`pnp-webpack-plugin`);
 const webpack = require('webpack');
 
 module.exports = {
@@ -11,8 +12,20 @@ module.exports = {
       }
     ]
   },
+  resolveLoader: {
+    plugins: [
+      PnpWebpackPlugin.moduleLoader(module),
+    ],
+  },
   resolve: {
-    extensions: ['*', '.js', '.jsx']
+    extensions: ['*', '.js', '.jsx'],
+    plugins: [
+      PnpWebpackPlugin,
+    ],
+    modules: [
+      `node_modules`,
+      `${__dirname}/src`
+    ]
   },
   output: {
     path: __dirname + '/dist',

[arcanis]

That's strange - I've just added a test and it seems to work 😮

https://github.com/arcanis/pnp-webpack-plugin/blob/master/index.test.js#L56-L61

Will try to get a repro with your repository

[arcanis]

Ok! I understood why it worked - an interaction between Jest and an edge case in PnP was causing that. I've setup yarnpkg/yarn#6643 to fix that.

Regarding the modules problem, I'll think a bit more and make a decision tomorrow - it annoys me to disable the semantic errors on Webpack, though ... maybe a good compromise would be to only allow errors to go through when specific options are detected (such as modules).

[bugzpodder]

I like your suggestion. Perhaps we can check if !(modules.length === 1 && modules[0] == '/node_modules/').

[arcanis]

Yep, something like that would be good! 👍

[bugzpodder]

Unfortunately i spent a few minutes digging into it and I couldn't figure out how (disclaimer: I am a webpack plugin n00b). Feel free to close this PR and make your own that does all these!