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

Webpak - Critical dependency: the request of a dependency is an expression #199

Closed
frandiox opened this issue Dec 7, 2017 · 11 comments
Closed

Comments

@frandiox
Copy link

frandiox commented Dec 7, 2017

Hi! I want to bundle postcss-cssnext package with Webpack for a web app and browserslist is also included. I'm following the information mentioned in README.md to remove locales (new webpack.IgnorePlugin(/caniuse-lite\/data\/regions/)) but have a doubt on something else.

There's a scary warning when bundling with Webpack: Critical dependency: the request of a dependency is an expression on this line. According to Webpack docs, full dynamic requires fail by default.

They mention that ContextReplacementPlugin can be used in this situation but I don't really know what is the actual context and where to redirect it. Any idea on how to fix this?

Thanks!

@ai
Copy link
Member

ai commented Dec 7, 2017

Yeap, I understood the problem, but don’t know solution right now.

I will try to find some ideas on this weekend.

@ai
Copy link
Member

ai commented Dec 7, 2017

@TheLarkInn hi. Can I ask you advice here?

Browserslist is used in Autoprefixer and many other tools. If somebody wants to have something like CodePen in the browser, it will load Browserslist.

We have require(variable) in the Browserslist. It requires absolutely dynamic path. But it is not so important for web users. It will be enough just to hide this require from Webpack.

Do we have something like require(/* ignore */ variable)?

@ai
Copy link
Member

ai commented Dec 7, 2017

@frandiox as a temporary solution you can use resolve.dynamic: false in the webpack config.

@frandiox
Copy link
Author

frandiox commented Dec 7, 2017

@ai Thanks for answering! Looks like resolve.dynamic: false is not a valid option at least in webpack@3.10.0. The following suppresses the warning but I'm not sure if it's reliable:

module: {
  exprContextRegExp: /$^/,
  exprContextCritical: false,
}

According to the docs, these props are deprecated and ContextReplacementPlugin should be used instead. However, I couldn't stop the warning with that plugin... 😕

@acusti
Copy link
Contributor

acusti commented Jan 2, 2018

@frandiox Thanks for posting that! I looked into using the ContextReplacementPlugin instead of those config options, but I’m now fairly confident that the deprecation notice / recommendation in the docs doesn’t apply to this situation, because in this case, the var queries = require(name) doesn’t actually result in webpack creating a context, because it is only an expression, and it’s impossible to replace a context that never gets created. This case is specifically mentioned in this gist (which accompanies this article), along with the following explanation:

When you specify only a single variable, there isn’t enough information for webpack to create a context.

Considering that, I think your config example is the correct solution, though I made it exprContextRegExp: /^$/, because I found it strange to see the end-of-line and start-of-line markers out of order. Perhaps the implications of that deprecation warning for situations like these should be brought up as an issue in the webpack repo if it hasn’t already.

Separately, while exploring using ContextReplacementPlugin, I discovered that another dynamic require in browserslist, on this line, was causing 560 KB of caniuse-lite regions data to be included in my build. I don’t use any regions-based browser queries or feature detection in our app (browserslist only gets included in our app by postcss/autoprefixer, which we run in the browser), so I added the following to my webpack config and decreased my bundle size by over half a meg:

    plugins: [
        // No need for any caniuse regions data (for require context in browserslist)
        new webpack.ContextReplacementPlugin(
            /caniuse-lite[\/\\]data[\/\\]regions/,
            /^$/,
        ),
    ],

@ai
Copy link
Member

ai commented Jan 3, 2018

@acusti we even have it in docs https://github.com/ai/browserslist#webpack

Did we found solution? What I need to add to docs?

@lifeiscontent
Copy link

lifeiscontent commented Jan 12, 2018

@ai why not just require the caniuse-lite if the context is a browser? that way we don't need to edit our config, in my case, I'm using create-react-app so I can't edit the webpack config, and in previous versions of create-react-app, it just worked out of the box, but I guess since upgrading webpack it has broke.

My suggestion here is to check if window exists, if it does, serve the lite version.

@ai
Copy link
Member

ai commented Jan 13, 2018

@lifeiscontent hm, good idea. But better way is to use package.browser. I will try to implement it tomorrow.

@lifeiscontent
Copy link

@ai sounds good! 👍

@ai
Copy link
Member

ai commented Jan 15, 2018

Done 03c72ca

@ai
Copy link
Member

ai commented Jan 15, 2018

Released in 2.11.2

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

4 participants