Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolving root path in RootsPlugin #308

Open
bryanchen-d opened this issue Sep 24, 2021 · 11 comments
Open

Resolving root path in RootsPlugin #308

bryanchen-d opened this issue Sep 24, 2021 · 11 comments

Comments

@bryanchen-d
Copy link

bryanchen-d commented Sep 24, 2021

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:

.tapAsync("RootsPlugin", (request, resolveContext, callback) => {
const req = request.request;
if (!req) return callback();
if (!req.startsWith("/")) return callback();
forEachBail(
this.roots,
(root, callback) => {
const path = resolver.join(root, req.slice(1));

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.
@alexander-akait
Copy link
Member

Can you provide example how we can reproduce it?

@bryanchen-d
Copy link
Author

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 webpack@5.53.0, react-dev-utils@11.0.3, 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.

@alexander-akait
Copy link
Member

Yep, feel free to ping me

@bryanchen-d
Copy link
Author

Yep, feel free to ping me

Updated with a repro, please check.

@alexander-akait
Copy link
Member

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

@bryanchen-d
Copy link
Author

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.

@alexander-akait
Copy link
Member

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

@bryanchen-d
Copy link
Author

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.

@alexander-akait
Copy link
Member

Note - you can have multiples roots

@bryanchen-d
Copy link
Author

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?

@alexander-akait
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants