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

webpack warning: "Critical dependency: the request of a dependency is an expression" #4959

Closed
eranation opened this issue Aug 9, 2018 · 24 comments
Assignees
Labels
area:standalone Issues with Prettier's Standalone build (meant to be used in the browser) locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. status:has pr Issues with an accompanying pull request. These issues will probably be fixed soon! type:bug Issues identifying ugly output, or a defect in the program
Milestone

Comments

@eranation
Copy link

Environments:

  • Prettier Version: 1.14.2
  • Running Prettier via: Angular 6, WebPack, TypeScript
  • Runtime: Node v8.9.4, TypeScript 2.7.2
  • Operating System: macOS

Steps to reproduce:

npm install --save --save-exact prettier

in some component

import * as prettier from "prettier/standalone";
import * as parserYaml from "prettier/parser-yaml";

...

//somewhere within 
const formatted = prettier.format(..., { semi: false, parser: "yaml", plugins:[parserYaml] } );

Expected behavior:

No compilation warnings

Actual behavior:

WARNING in ./node_modules/prettier/standalone.js
7397:15-64 Critical dependency: the request of a dependency is an expression

(everything seems to work well, the issue is just the compilation warning)

@lydell
Copy link
Member

lydell commented Aug 9, 2018

Hi! Can you make a repo that reproduces the problem? That would be really helpful!

@lydell lydell added the status:awaiting response Issues that require answers to questions from maintainers before action can be taken label Aug 9, 2018
@philcockfield
Copy link

philcockfield commented Aug 20, 2018

Here's a sample repo demonstrating the problem: https://github.com/philcockfield/prettier-sample

prettier

WARNING in ./node_modules/prettier/standalone.js
7397:15-64 Critical dependency: the request of a dependency is an expression
 @ ./node_modules/prettier/standalone.js
 @ ./lib/common/prettier.js
 @ ./lib/common/index.js
 @ ./lib/test/storybook.js
 @ ./lib/components/Foo.stories.js
 @ ./lib stories.js$
 @ ./.storybook/config.js
 @ multi ./node_modules/@storybook/react/dist/server/config/polyfills.js ./node_modules/@storybook/react/dist/server/config/globals.js (webpack)-hot-middleware/client.js?reload=true ./.storybook/config.js
Child html-webpack-plugin for "index.html":
                                   Asset      Size  Chunks             Chunk Names
                              index.html    569 kB       1
    7b056e65e12fac1b83ba.hot-update.json  44 bytes          [emitted]
       [0] ./node_modules/html-webpack-plugin/lib/loader.js!./node_modules/@storybook/react/dist/server/index.html.ejs 1.7 kB {1}
       [1] ./node_modules/lodash/lodash.js 540 kB {1}
       [2] (webpack)/buildin/global.js 509 bytes {1}
       [3] (webpack)/buildin/module.js 517 bytes {1}
Child html-webpack-plugin for "iframe.html":
                                   Asset      Size  Chunks             Chunk Names
                             iframe.html    569 kB       1
    63d0aee88fe26bae2acc.hot-update.json  44 bytes          [emitted]
       [0] ./node_modules/html-webpack-plugin/lib/loader.js!./node_modules/@storybook/react/dist/server/iframe.html.ejs 1.16 kB {1}
       [1] ./node_modules/lodash/lodash.js 540 kB {1}
       [2] (webpack)/buildin/global.js 509 bytes {1}
       [3] (webpack)/buildin/module.js 517 bytes {1}

@lydell
Copy link
Member

lydell commented Aug 20, 2018

Awesome! Is there any chance you could make the example smaller? For example, get rid of Storybook and Typescript? I think that would increase your chances of this getting fixed.

@lydell lydell added help wanted We're a small group who can't get to every issue promptly. We’d appreciate help fixing this issue! and removed status:awaiting response Issues that require answers to questions from maintainers before action can be taken labels Aug 20, 2018
@philcockfield
Copy link

philcockfield commented Aug 20, 2018

Here are details on what that error is and means within Webpack:
webpack/webpack#196 (comment)

And it looks like a work around to this, prior to it being fixed within Prettier itself, is to use ContextReplacementPlugin as per this comment


It looks like something in prettier/standalone.js is dynamically generating a path string that is passed to a require statement.


Hopefully that gives something more to go on.

@lydell
Copy link
Member

lydell commented Aug 21, 2018

Might be this line:

$ grep -n 'require(' standalone.js 
7393:        parse: require(path.resolve(process.cwd(), opts.parser)),

@philcockfield
Copy link

philcockfield commented Aug 21, 2018 via email

@lydell lydell changed the title Critical dependency: the request of a dependency is an expression webpack warning: "Critical dependency: the request of a dependency is an expression" Aug 21, 2018
@nickforddev
Copy link

@philcockfield That thread says nothing about how to actually implement a workaround, just a vague comment about maybe using ContextReplacementPlugin. I'm just putting it out there so people don't waste their time skimming the entire thread

@philcockfield
Copy link

@nickarnold - cool, I haven't figured out how to use that ContextReplacementPlugin yet - but now we can see the specific line that is causing the fail, feels like a quick/easy fix (as found by @lydell )

$ grep -n 'require(' standalone.js 
7393:        parse: require(path.resolve(process.cwd(), opts.parser)),

@philcockfield
Copy link

philcockfield commented Aug 29, 2018

Confirmed that if the following two lines in the build standalone.js file are commented out, webpack does not throw this error - so it looks like it's just these two call-sites that need to somehow pass in an explicit path to require, or maybe do something other than use require to get these files included.

7393:        parse: require(path.resolve(process.cwd(), opts.parser)),

Location in source code

and

                case "TSExternalModuleReference":
14793        return concat$4(["require(", path.call(print, "expression"), ")"]);

Location in source code

@lydell
Copy link
Member

lydell commented Aug 29, 2018

webpack does not throw this error

It's a warning, isn't it?


Are you sure that line 14793 causes a warning? That sounds strange to me. That line is basically just building a string containing the word "require" – surely that has to be allowed by webpack?

1 similar comment
@lydell

This comment has been minimized.

@philcockfield
Copy link

Damn, my bad on both accounts @lydell. Just re-tested and it is just line 7393 that if commented out stops the "warning" (not "error")... yes. Apologies for confusion.

        // parse: require(path.resolve(process.cwd(), opts.parser)),

@philcockfield
Copy link

philcockfield commented Aug 29, 2018

I've just figured out I can suppress this with this webpack plugin, so I'll do that for now. Mentioning here if anyone else it trying to do the same.

  config.plugins.push(
    new FilterWarningsPlugin({
      exclude: /Critical dependency: the request of a dependency is an expression/,
    }),
  );

As @lydell points out, it's a "warning" from Webpack, not an error, but man Webpack really does scream blue-murder about this in the console, which makes it hard to disambiguate from actual errors when developing.

@j-f1
Copy link
Member

j-f1 commented Sep 1, 2018

@hedgepigdaniel
Copy link

It doesn't make sense to have this line of code

parse: eval("require")(path.resolve(process.cwd(), opts.parser)),
being compiled to run in a browser (where there is no process.cwd). Webpack warns because that statement is implicitly asking it to compile all the code that could possibly be imported for any value of process.cwd() and opts.parser. Seeing as that's impossible I assume it gives up and lets it crash.

If you were compiling some node project that used prettier, and you wanted to use the functionality to specify a custom parser, you would probably want to pass it as a node API option, not as a CLI option.

Perhaps it makes sense for the main entry point not to import the code that parses the CLI --parser option and resolves it relative to CWD, so that that line is not included in projects that use prettier programmatically. Instead, only the binary entry point can import and use that code, and pass the resolved parser through to the node API.

@suchipi suchipi added type:bug Issues identifying ugly output, or a defect in the program area:standalone Issues with Prettier's Standalone build (meant to be used in the browser) labels Sep 5, 2018
secondsun added a commit to secondsun/data-sync-ui that referenced this issue Sep 7, 2018
There is an warning that is cause by one of the dependencies, the upstream bug is prettier/prettier#4959
secondsun added a commit to secondsun/data-sync-ui that referenced this issue Sep 7, 2018
There is an warning that is cause by one of the dependencies, the upstream bug is prettier/prettier#4959
@camdagr8
Copy link

This happens because Webpack doesn't actually use require() as a function but a RegEx, so the Node usage of require() as a file reader doesn't work in Webpack.

This should be taken out for the stand alone version.

@duailibe
Copy link
Member

PRs welcome

@j-f1
Copy link
Member

j-f1 commented Nov 14, 2018

Proposal:

  • Wrap the affected try block in an if (!process.env.PRETTIER_STANDALONE).
  • Create a Babel plugin that replaces if (!process.env.PRETTIER_STANDALONE) { ... } with its contents in regular mode and removes the whole statement in standalone mode.

@j-f1 j-f1 removed the help wanted We're a small group who can't get to every issue promptly. We’d appreciate help fixing this issue! label Dec 8, 2018
@j-f1 j-f1 self-assigned this Dec 8, 2018
@j-f1 j-f1 added the status:has pr Issues with an accompanying pull request. These issues will probably be fixed soon! label Dec 9, 2018
@j-f1
Copy link
Member

j-f1 commented Dec 12, 2018

If you’re currently experiencing this warning, can you try installing prettier/prettier instead of prettier to make sure my fix works?

@ikatyang
Copy link
Member

@j-f1 prettier/prettier is not bundled. (The bundled dist will be available on Azure Pipelines after #5611 merged, see here for example.)

@camdagr8
Copy link

camdagr8 commented Dec 13, 2018

Good job and thanks for the fix!

@j-f1
Copy link
Member

j-f1 commented Dec 13, 2018

Ah, good point @ikatyang.

@tfrijsewijk
Copy link

When is this slated for release?

@j-f1
Copy link
Member

j-f1 commented Dec 18, 2018

Probably sometime in January.

@ikatyang ikatyang added this to the 1.16 milestone Jan 9, 2019
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Apr 9, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Apr 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area:standalone Issues with Prettier's Standalone build (meant to be used in the browser) locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. status:has pr Issues with an accompanying pull request. These issues will probably be fixed soon! type:bug Issues identifying ugly output, or a defect in the program
Projects
None yet
Development

No branches or pull requests