enhanced-resolve icon indicating copy to clipboard operation
enhanced-resolve copied to clipboard

Resolving root path in RootsPlugin

Open bryanchen-d opened this issue 4 years ago • 11 comments

Hi, while I am tracing down an issue from react-dev-utils/ModuleScopePlugin, I found that line 37 in RootsPlugin is causing different behavior between linux and windows as it only check for the / path separator: https://github.com/webpack/enhanced-resolve/blob/d1a3cb65cdf90754871a263172006e2902a2f86e/lib/RootsPlugin.js#L34-L42

Also I am curious about when the request path is an absolute path starting with '/', why we want to trim off the '/' and treat it as a relative path and concatenate it with another root path?

---- A simple setup to repro the issue

  1. Follow https://webpack.js.org/guides/getting-started/ to setup a simple env of webpack
  2. in index.js add some code using crypto like
import crypto from "crypto";
console.log(crypto.createHash("sha256").digest("hex"));
  1. Add a webpack.config.js with content
const path = require('path');
const ModuleScopePlugin = require("react-dev-utils/ModuleScopePlugin");
module.exports = {
  entry: './src/index.js',
  resolve:{
      fallback:{
        crypto:require.resolve("crypto-browserify"),
        buffer:require.resolve("buffer"),
        stream: require.resolve("stream-browserify")
      },
      plugins: [
        new ModuleScopePlugin(path.resolve(__dirname, "./src"))
      ],
  },
  output: {
    filename: 'main.js',
    path: path.resolve(__dirname, 'dist'),
  }
};
  1. Build by running npx webpack

Then I am seeing

**ERROR** in ./src/index.js 2:0-28
Module not found: Error: You attempted to import /home/brchen/repos/webpack-test/node_modules/crypto-browserify/index.js which falls outside of the project src/ directory. Relative imports outside of src/ are not supported.

bryanchen-d avatar Sep 24 '21 16:09 bryanchen-d

Can you provide example how we can reproduce it?

alexander-akait avatar Sep 24 '21 16:09 alexander-akait

Can you provide example how we can reproduce it?

Thanks for the quick turnaround! I don't have a simple repro for now as my project's webpack config involves quite some plugins. Some info for the issue: I am working on upgrading to [email protected], [email protected], and there is a fallback config for crypto: resolve.fallback.crypto=require.resolve("crypto-browserify"). It builds well for me on windows, but it failed on linux that the ModuleScopePlugin throws error complaining the /home/.../node_modules/crypto-browserify/index.js is outside of the ../src/ folder.

I will see if I can get a simple setup to repro the issue and update here. Thanks again.

bryanchen-d avatar Sep 24 '21 17:09 bryanchen-d

Yep, feel free to ping me

alexander-akait avatar Sep 24 '21 17:09 alexander-akait

Yep, feel free to ping me

Updated with a repro, please check.

bryanchen-d avatar Sep 24 '21 17:09 bryanchen-d

hm, looks line ModuleScopePlugin should ignore request to node_modules, but why do you use this plugin (i.e. why CRA uses), we have the restrictions options, it can be used instead this plugin

alexander-akait avatar Sep 24 '21 17:09 alexander-akait

Oh good to know that, the project was originally created with create-react-app which brings in the ModuleScopePlugin at the beginning, I can try out the new setting.

Regarding the error, from what I found, the ModuleScopePlugin did try to ignore those requests as in https://github.com/facebook/create-react-app/blob/5cedfe4d6f19a5f07137ac7429f57dfe923d93b5/packages/react-dev-utils/ModuleScopePlugin.js#L30-L38

However the RootsPlugin returns a wrong path after it concatenates two root paths, and then later resolver cannot find the description file (package.json) of the 'crypto-browserify' package, that seems to set the descriptionFileRoot back to the project folder, which escape ModuleScopePlugin's check above.

bryanchen-d avatar Sep 24 '21 17:09 bryanchen-d

Maybe ModuleScopePlugin's plugin should use resolved hook https://github.com/webpack/enhanced-resolve/blob/main/lib/ResolverFactory.js#L303... Anyway if you know how to fix feel free to send a fix

alexander-akait avatar Sep 24 '21 17:09 alexander-akait

For the build error I have workarounds anyway. For a fix here in RootsPlugin it seems to me that it should simply return callback() when the request.path is already a rooted path, so I am trying to understand the reasons of existing logic.

bryanchen-d avatar Sep 24 '21 18:09 bryanchen-d

Note - you can have multiples roots

alexander-akait avatar Sep 24 '21 18:09 alexander-akait

You mean multiple roots as in this.roots ? But regardless what roots we have, I think in this case the request has specified a rooted path, so actually we should just honor that, right?

bryanchen-d avatar Sep 24 '21 18:09 bryanchen-d

Need check, just information :smile: I can look at this at weekends, feel free to ping me if you don't found solution

alexander-akait avatar Sep 24 '21 18:09 alexander-akait