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 critical dependency warning #334

Closed
ilg-ul opened this issue Apr 14, 2021 · 15 comments
Closed

webpack critical dependency warning #334

ilg-ul opened this issue Apr 14, 2021 · 15 comments

Comments

@ilg-ul
Copy link

ilg-ul commented Apr 14, 2021

Hi,

I'm successfully using liquidjs in my projects for a while, but today I encountered a small issue while trying to pack it in a VSCode extension, webpack reported the following:

ilg@wks vscode-xpack-extension-ts.git % npm run publish

> [email protected] publish
> vsce publish

Executing prepublish script 'npm run vscode:prepublish'...

> [email protected] vscode:prepublish
> webpack --mode production

asset extension.js 89.2 KiB [compared for emit] [minimized] (name: main) 2 related assets
runtime modules 937 bytes 4 modules
modules by path ./ 221 KiB
  modules by path ./src/ 65.3 KiB 11 modules
  modules by path ./node_modules/ 155 KiB
    modules by path ./node_modules/@xpack/logger/ 12.2 KiB 2 modules
    modules by path ./node_modules/make-dir/ 44.2 KiB 2 modules
    modules by path ./node_modules/liquidjs/dist/ 93.7 KiB 2 modules
    ./node_modules/vscode-cpptools/out/api.js 5.34 KiB [built] [code generated]
external "assert" 42 bytes [built] [code generated]
external "vscode" 42 bytes [built] [code generated]
external "fs" 42 bytes [built] [code generated]
external "os" 42 bytes [built] [code generated]
external "path" 42 bytes [built] [code generated]
external "util" 42 bytes [built] [code generated]
external "process" 42 bytes [built] [code generated]

WARNING in ./node_modules/liquidjs/dist/liquid.node.esm.js 254:15-36
Critical dependency: the request of a dependency is an expression
    at RequireResolveContextDependency.getWarnings (/Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/webpack/lib/dependencies/ContextDependency.js:82:18)
    at Compilation.reportDependencyErrorsAndWarnings (/Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/webpack/lib/Compilation.js:2525:24)
    at /Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/webpack/lib/Compilation.js:2156:10
    at _next2 (eval at create (/Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:34:1)
    at eval (eval at create (/Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:62:1)
    at /Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/webpack/lib/FlagDependencyExportsPlugin.js:352:11
    at /Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/neo-async/async.js:2830:7
    at Object.each (/Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/neo-async/async.js:2850:39)
    at /Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/webpack/lib/FlagDependencyExportsPlugin.js:331:18
    at /Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/neo-async/async.js:2830:7
 @ ./src/lib/xpm-liquid.ts 22:19-38
 @ ./src/lib/data-model.ts 24:21-44
 @ ./src/lib/manager.ts 19:21-44
 @ ./src/extension.ts 17:18-42

webpack 5.32.0 compiled with 1 warning in 3281 ms
Publishing [email protected]...
 INFO  Extension URL (might take a few minutes): https://marketplace.visualstudio.com/items?itemName=ilg-vscode.xpack
 INFO  Hub URL: https://marketplace.visualstudio.com/manage/publishers/ilg-vscode/extensions/xpack/hub
 DONE  Published [email protected].
ilg@wks vscode-xpack-extension-ts.git % 

In line 254 of liquid.node.esm.js there is require object, which apparently confuses webpack:

function fallback(file) {
    try {
        return require.resolve(file);
    }
    catch (e) { }
}

The webpack documentation warns that these messages should be taken seriously:

I'm quite new to webpack/VSCode extensions/TypeScript, so I don't have a solution, but it would be great to have a webpack friendly liquidjs, in order to avoid workarounds like having to exclude liquidjs from web bundles.

Thank you in advance,

Liviu

@harttle
Copy link
Owner

harttle commented Apr 18, 2021

After digging into the issue, I find it imported via #168 which requests a template lookup feature (just like nunjucks) via require.resolve. FYI: it's located inside this environment-dependent file:

return require.resolve(file)

Meaning:

  1. it's only included in the cjs bundle for Node.js so its assumption about require being available to use is correct.
  2. it's a dynamic feature by design to lookup templates rather than JavaScript files.

With that being said, you can safely suppress or ignore that warning.

@harttle harttle closed this as completed Apr 18, 2021
@ilg-ul
Copy link
Author

ilg-ul commented Apr 18, 2021

you can safely suppress or ignore that warning

Ok, thank you for clarifying this.

Perhaps you can also document it in the project web or README.

@ilg-ul
Copy link
Author

ilg-ul commented Apr 19, 2021

@harttle, I received a comment from the webpack team, they suggest to use a different solution to solve dynamic dependencies, via createRequire():

@harttle
Copy link
Owner

harttle commented Apr 19, 2021

It's added in v12.2 thus not available for liquidjs. But I'm willing to update when Node.js 12.2 become the lowest LTS.

@ilg-ul
Copy link
Author

ilg-ul commented Apr 19, 2021

Sounds good. In my other projects I also continue to use 10.x, so I think this is reasonable.

@ilg-ul
Copy link
Author

ilg-ul commented Apr 19, 2021

Perhaps you should open a separate ticket to keep track of this new issue related to using createRequire() after updating to Node.js 12.

@ilg-ul
Copy link
Author

ilg-ul commented Apr 17, 2022

I updated my projects to use Node 12, liquidjs to 9.36.0, and generally everything is up to date.

However I still get the warning:

WARNING in ./node_modules/liquidjs/dist/liquid.node.esm.js 393:15-36
Critical dependency: the request of a dependency is an expression
    at RequireResolveContextDependency.getWarnings (/Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/webpack/lib/dependencies/ContextDependency.js:91:18)
    at Compilation.reportDependencyErrorsAndWarnings (/Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/webpack/lib/Compilation.js:3132:24)
    at /Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/webpack/lib/Compilation.js:2729:28
    at _next2 (eval at create (/Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:16:1)
    at eval (eval at create (/Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:42:1)
    at /Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/webpack/lib/FlagDependencyExportsPlugin.js:385:11
    at /Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/neo-async/async.js:2830:7
    at Object.each (/Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/neo-async/async.js:2850:39)
    at /Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/webpack/lib/FlagDependencyExportsPlugin.js:361:18
    at /Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/neo-async/async.js:2830:7

harttle added a commit that referenced this issue Apr 17, 2022
github-actions bot pushed a commit that referenced this issue Apr 17, 2022
## [9.36.1](v9.36.0...v9.36.1) (2022-04-17)

### Bug Fixes

* contains operator does not support Drop, fixes [#492](#492) ([9e024ff](9e024ff))
* responsive header ([a56af6b](a56af6b))
* use `createRequire` for ESM, fixes [#334](#334) ([eec381e](eec381e))
@github-actions
Copy link

🎉 This issue has been resolved in version 9.36.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ilg-ul
Copy link
Author

ilg-ul commented Oct 21, 2022

I updated my project to refer to 9.42.0, but webpack still complains:

WARNING in ./node_modules/liquidjs/dist/liquid.node.esm.js 263:9-30
Critical dependency: the request of a dependency is an expression
    at RequireResolveContextDependency.getWarnings (/Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/webpack/lib/dependencies/ContextDependency.js:102:18)
    at Compilation.reportDependencyErrorsAndWarnings (/Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/webpack/lib/Compilation.js:3132:24)
    at /Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/webpack/lib/Compilation.js:2729:28
    at _next2 (eval at create (/Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:16:1)
    at eval (eval at create (/Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:42:1)
    at /Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/webpack/lib/FlagDependencyExportsPlugin.js:385:11
    at /Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/neo-async/async.js:2830:7
    at Object.each (/Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/neo-async/async.js:2850:39)
    at /Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/webpack/lib/FlagDependencyExportsPlugin.js:361:18
    at /Users/ilg/My Files/WKS Projects/xpack.github/vscode-extensions/vscode-xpack-extension-ts.git/node_modules/neo-async/async.js:2830:7
 @ ./node_modules/@xpack/xpm-liquid/dist/lib/xpm-liquid.js 27:19-38
 @ ./node_modules/@xpack/xpm-liquid/dist/index.js 27:13-40
 @ ./src/lib/data-model.ts 24:21-49
 @ ./src/lib/manager.ts 21:21-44
 @ ./src/extension.ts 21:18-42

webpack 5.74.0 compiled with 1 warning in 2077 ms

What might be wrong?

@harttle harttle reopened this Oct 21, 2022
@harttle
Copy link
Owner

harttle commented Oct 21, 2022

I'll try to create a demo repo for this, will ping back if I find something.

@ilg-ul
Copy link
Author

ilg-ul commented Oct 21, 2022

Sure, please let me know if there is anything I can do to help you.

@harttle
Copy link
Owner

harttle commented Oct 21, 2022

This "request of a dependency is an expression" is thrown because LiquidJS has a feature to resolve template partials from node_modules. For example, the following hello-liquid is a npm dependency instead of an actual file:

<h1>{% include "hello-liquid" %}</h1>

But in webpack, require will be compiled to __webpack_require__ and is no longer working. So webpack prints this warning for it. There's no difference we use require or createRequire, webpack knows it well.

I think it's kind of expected. Maybe we need figuire out how to suppress it if not revelant to the final repo, or how to config webpack to make it working if it's indeed useful. I think documentation effort is enough for this issue, how do you think?

@ilg-ul
Copy link
Author

ilg-ul commented Oct 21, 2022

I don't know, this seems a bit too technical for me.

From a user point of view, the warning thrown by webpack is a concern; you, as the author, can asses that it is not harmful, but me, as the less knowledgeable user whose general strategy is to avoid all warnings, can not, so whenever I see it I'm concerned. :-(

So, if you, together with the webpack team, can find a solution to suppress this warning, it would be great.

@harttle
Copy link
Owner

harttle commented Oct 21, 2022

"loading partials from node modules" is no longer supported in LiquidJS when compiled by webpack. webpack tries to pack all dependencies (including ones in node_modules) into its output bundles, which is against the idea of “resolving by node's require & reading the file as template". (though one can create a sophisticated webpack loader for this purpose)

Adding the following into your webpack config will suppress this warning:

  // Here's a demo for using Liquid with webpack
  // https://github.com/harttle/liquidjs/tree/master/demo/webpack
  plugins: [
    new (require('webpack').ContextReplacementPlugin)(/liquidjs/)
  ],

@harttle harttle closed this as completed Oct 21, 2022
@ilg-ul
Copy link
Author

ilg-ul commented Oct 21, 2022

I confirm that with this definition added to webpack.config.js, the warning is no longer triggered.

Thank you very much!

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

No branches or pull requests

2 participants