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

This crate hits a buggy Webpack 5 warning when building for wasm32-unknown #224

Closed
josephlr opened this issue Sep 1, 2021 · 4 comments
Closed
Assignees

Comments

@josephlr
Copy link
Member

josephlr commented Sep 1, 2021

See this comment for context: #137 (comment)

Previous toil on this issue:
Fix in this crate for Webpack 4: #137
Rustwasm bug: rustwasm/wasm-pack#822
Webpack bug closed as WONTFIX: webpack/webpack#8826
Advice on how to fix this issue for Webpack: webpack/webpack#8826 (comment)

Basically, our #[wasm_bindgen] declaration generates Javascript that looks like:

module.require(getStringFromWasm0(arg0, arg1));

Despite this being valid JavaScript, Webpack doesn't like it for some reason.

It seems like our options to fix this are:

  • Tell users how to ignore the warning (it is just a warning after all)
  • Tell users how to configure Webpack to not get this error (involves some package.json stuff I don't understand)
  • File an upstream bug with Webpack to properly support wasm
  • File an upstream bug with the Rust wasm_bindgen tool to annotate module.require with some sort of "ignore warning" requirement
  • Use a Node.js dynamic import() call
  • Use wasm_bindgen's module attribute.
  • Use wasm_bindgen's inline_js attribute
  • Get upstream Rust to finally have wasm32-unknown-browser and wasm32-unknown-node targets, so that we don't have to do browser vs Node detection at runtime.

Things we shouldn't try to do:

  • Replace this with another hack to trick the bundler (I don't want to keep dealing with this issue)
  • eval("require(crypto)")

However, I don't have the expertise to solve this problem. If anyone (@Pauan, @huacnlee , @Herohtar) knows how to fix this issue, I would happily accept a PR provided the solution is documented and our CI keeps passing (we don't use the Webpack bundler in our CI, hence why we missed this).

@mjhanninen
Copy link
Contributor

In order to tell Webpack 5 to ignore this specific warning you can add the following into your webpack.config.js:

module.exports = {
  // ...
  ignoreWarnings: [
    (warning) =>
      warning.message ===
      "Critical dependency: the request of a dependency is an expression",
  ],
  // ...
}

Naturally this is not the proper fix for the problem; just a work-around in case the warning is producing too much nuisance for you.

@mjhanninen
Copy link
Contributor

I just opened the PR #234 for addressing this problem.

Here's my .02$ on the alternatives listed in the issue description:

  • "Tell users how to ignore the warning (it is just a warning after all)": In my case Webpack development server's warning overlay effectively blocked the whole browser window. This was terrible experience.
  • "Tell users how to configure Webpack to not get this error (involves some package.json stuff I don't understand)": This worked for me.
  • "File an upstream bug with Webpack to properly support wasm": As far as I understand this is not a question of WASM support as such but more about how Webpack determines the set packages it should bundle and what information it has available during bundling. I think Webpack operates sensibly.
  • "File an upstream bug with the Rust wasm_bindgen tool to annotate module.require with some sort of "ignore warning" requirement": Yes, I think wasm-bindgen should have a mechanism to this effect.
  • "Use a Node.js dynamic import() call": Not supported by wasm-bindgen according to the docs.
  • "Use wasm_bindgen's module attribute": Not supported on NodeJS target according to the wasm-bindged docs. Also related.
  • "Use wasm_bindgen's inline_js attribute": Ditto.
  • "Get upstream Rust to finally have wasm32-unknown-browser and wasm32-unknown-node targets, so that we don't have to do browser vs Node detection at runtime": I'd like hat.

@yokomizor
Copy link

Despite this being valid JavaScript, Webpack doesn't like it for some reason.

I think the reason is that webpack needs to know the value upfront, is to apply the correct loader if necessary. Users can define rules based on this value to use specific loaders such as ts-loader for example. There are loaders, for example, that inline the import, replacing it by the contents of the module: https://github.com/facebook/create-react-app/blob/edc671eeea6b7d26ac3f1eb2050e50f75cf9ad5d/packages/react-dev-utils/InlineChunkHtmlPlugin.js

I guess If the dependency is an expression, webpack will just leave it as is and skip the loader rules.

mosmeh added a commit to mosmeh/beek that referenced this issue Feb 10, 2022
- Use production mode for wasm-pack
Development mode results in 'Unexpected section' error,
possibly due to unstripped sections in wasm.

- Ignore "Critical dependency" warning
The problem and the workaround are described in
rust-random/getrandom#224
@josephlr
Copy link
Member Author

Closing as #234 and #284 solve this problem.

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

3 participants