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 compat error for wasm files imported in web workers #22581

Closed
nickbabcock opened this issue Feb 26, 2021 · 12 comments
Closed

Webpack compat error for wasm files imported in web workers #22581

nickbabcock opened this issue Feb 26, 2021 · 12 comments
Labels
bug Issue was opened via the bug report template.

Comments

@nickbabcock
Copy link
Contributor

What version of Next.js are you using?

10.0.7

What version of Node.js are you using?

v14.3.0

What browser are you using?

N/A

What operating system are you using?

Ubuntu 20.04

How are you deploying your application?

N/A

Describe the Bug

I took the with-webassembly example and moved the loading and execution of wasm into a web worker (to simulate offloading of expensive functions to wasm in another thread). Unfortunately I receive the following error for both next dev and next build

Failed to compile.

webpack/runtime/compat
The "path" argument must be of type string. Received undefined

Which I have been unable to debug.

The project is straightforward (no additional dependencies, just plain next.js) but I'll copy the relevant bits here for posterity:

worker.js

onmessage = async function (e) {
  const rustModule = await import("../add.wasm");
  console.log(rustModule.add_one(10));
};

index.js

import { useEffect, useRef } from "react";

const Page = () => {
  const workerRef = useRef();
  useEffect(() => {
    if (workerRef.current === undefined) {
      workerRef.current = new Worker(new URL("./worker.js", import.meta.url));
    }
  }, []);

  return (
    <button onClick={() => workerRef.current.postMessage([])}>Click</button>
  );
};

export default Page;

One very important bit is that this is a webpack 5 project as webpack 5 supports web workers and wasm natively (albeit experimentally) and I've been unable to get webpack 4 working with the appropriate plugins due to #21679 .

Wasm files outside of web workers work, and web workers without wasm work, the combination of them seems to give rise to the error. It doesn't matter where the wasm module is imported in the web worker or even if it is dynamically imported or not.

Expected Behavior

For the project to be built

To Reproduce

I've created a bug repro: https://github.com/nickbabcock/next.js-wasm-worker

clone, install, and build to repro the error.

@nickbabcock nickbabcock added the bug Issue was opened via the bug report template. label Feb 26, 2021
@nickbabcock
Copy link
Contributor Author

I have found a workaround by leveraging webpack asset modules to instruct webpack and next to only return the path of the digested wasm. The downside is that now one has to write the code to fetch, compile, and instantiate which is not too hard. This is how I achieved it:

next.config.js

module.exports = {
  future: {
    webpack5: true
  },
  webpack: (config) => {
    const experiments = config.experiments || {};
    config.experiments = {...experiments, asyncWebAssembly: true};
    config.output.assetModuleFilename = `static/[hash][ext]`;
    config.output.publicPath = `/_next/`;
    config.module.rules.push({
      test: /\.wasm/,
      type: 'asset/resource',
    })
    return config
  },
}

worker.js

import wasmPath from "../add.wasm";
onmessage = async function (e) {
  const fetchPromise = fetch(wasmPath);
  const { instance } = await WebAssembly.instantiateStreaming(fetchPromise);
  console.log(instance.exports.add_one(10));
};

Look ma, no 3rd party dependencies.

I still believe there is a bug between how next.js and webpack communicate wam modules referenced by web workers, but at least the workaround is tolerable.

@nickbabcock
Copy link
Contributor Author

The use of webpack asset modules is a good enough workaround that I'm closing this issue

@tomlagier
Copy link

tomlagier commented Jun 19, 2021

This workaround doesn't work for me, unfortunately. I'm using wasm-pack and I need the "glue" it produces, which includes the wasm module import.

I've tracked the error down to this line: https://github.com/vercel/next.js/blob/canary/packages/next/build/webpack/plugins/nextjs-ssr-import.ts#L14

chunk.name is undefined. I believe what's happening is that the dynamic import plugin is incorrectly parsing the import statement, that should be getting handled by webpack alone. I'll see if there's some configuration I can pass that will achieve this but I'm not terribly hopeful of finding a fix without Next team help.

Here's chunk:
image

EDIT:

Just skipping the body of NextSSRImportPlugin if there is no chunk.name fixes the issue. I'm not sure what the implications of this are but I'm happy to open a PR just for discussion's sake.

tomlagier added a commit to tomlagier/next.js that referenced this issue Jun 19, 2021
This skips 'require' normalization for chunks without a name, which was throwing errors when used with WASM imports. See vercel#22581.
@tomlagier
Copy link

tomlagier commented Jun 19, 2021

To patch yourself without using the above PR, you can do this:

next.config.js

const SSRPlugin = require("next/dist/build/webpack/plugins/nextjs-ssr-import")
  .default;
 const { dirname, relative, resolve, join } = require("path");


module.exports = {
  webpack(config) {
     const ssrPlugin = config.plugins.find(
      plugin => plugin instanceof SSRPlugin
    );

    if (ssrPlugin) {
      patchSsrPlugin(ssrPlugin);
    }
  }
}

// Unfortunately there isn't an easy way to override the replacement function body, so we 
// have to just replace the whole plugin `apply` body.
function patchSsrPlugin(plugin) {
  plugin.apply = function apply(compiler) {
    compiler.hooks.compilation.tap("NextJsSSRImport", compilation => {
      compilation.mainTemplate.hooks.requireEnsure.tap(
        "NextJsSSRImport",
        (code, chunk) => {
          // This is the block that fixes https://github.com/vercel/next.js/issues/22581
          if (!chunk.name) {
            return;
          }

          // Update to load chunks from our custom chunks directory
          const outputPath = resolve("/");
          const pagePath = join("/", dirname(chunk.name));
          const relativePathToBaseDir = relative(pagePath, outputPath);
          // Make sure even in windows, the path looks like in unix
          // Node.js require system will convert it accordingly
          const relativePathToBaseDirNormalized = relativePathToBaseDir.replace(
            /\\/g,
            "/"
          );
          return code
            .replace(
              'require("./"',
              `require("${relativePathToBaseDirNormalized}/"`
            )
            .replace(
              "readFile(join(__dirname",
              `readFile(join(__dirname, "${relativePathToBaseDirNormalized}"`
            );
        }
      );
    });
  };
}

This is importing Next internals & mucking with them so it's likely to break in the future. Hopefully we can get that patch upstreamed :)

@joshrathke
Copy link

joshrathke commented Jun 23, 2021

We use the Worker interface provided by Webpack 5 and ran into this issue when attempting to update to NextJS 11. @tomlagier 's patch appears to have fixed it, but it would be great to not have to have that in our config file.

@stuartmemo
Copy link

Thanks for the fix @tomlagier!

Just for others - I had to add a return config to the above (in the webpack function) to avoid this error...
Error: Webpack config is undefined. You may have forgot to return properly from within the "webpack" method of your next.config.js..

@tom-sherman
Copy link

tom-sherman commented Jul 14, 2021

I have also received this error although not related to wasm

The error appears when I import a file from a worker that isn't a relative import. eg.

/pages/test.js:

new Worker(new URL("../worker.js", import.meta.url))

/worker.js:

// Non relative import
import * as Curry from "rescript/lib/es6/curry.js";

The config workaround above fixes the problem, as does the patch in #26372

tom-sherman added a commit to tom-sherman/rescript-lang.org that referenced this issue Jul 15, 2021
@Cyral
Copy link

Cyral commented Jul 18, 2021

@tom-sherman I am also receiving this error due to the web worker issue you describe (after upgrading to next 11). The fix by @tomlagier and patch #26372 work for me as well.

@donalffons
Copy link

donalffons commented Sep 11, 2021

In addition to @tomlagier's next.config.js changes, I had to include the following:

  webpack: (config, { buildId, dev, isServer, defaultLoaders, webpack }) => {
    // ...
    config.module.rules.find(k => k.oneOf !== undefined).oneOf.unshift(
      {
        test: /\.wasm$/,
        type: "asset/resource",
        generator: {
          filename: 'static/chunks/[path][name].[hash][ext]'
        },
      }
    );
    // ...

Without that bit, I was getting HTTP 404 errors during execution, because for some reason my wasm file was not properly served.

Now, everything seems to be working fine for me. I'm using an Emscripten-generated project using multithreading (i.e. web workers), compiled with EXPORT_ES6=1.

(Just noticed that this part of the solution was already mentioned by @nickbabcock. Took me a while to figure out that I have to combine both solutions)

@donalffons
Copy link

In addition to the error mentioned in this issue, I also got the error mentioned in #22813 (chunk names break in a production build, leading to HTTP 404 errors). Below is my full next.config.js, in case anyone else wants to use Emscripten-Generated projects with NextJS and Webpack 5. Most import Emscripten flags I use are -sEXPORT_ES6=1, -pthread, -sPTHREAD_POOL_SIZE='navigator.hardwareConcurrency', -sENVIRONMENT='web,worker'

const SSRPlugin = require("next/dist/build/webpack/plugins/nextjs-ssr-import").default;
const { dirname, relative, resolve, join } = require("path");

// Unfortunately there isn't an easy way to override the replacement function body, so we 
// have to just replace the whole plugin `apply` body.
function patchSsrPlugin(plugin) {
  plugin.apply = function apply(compiler) {
    compiler.hooks.compilation.tap("NextJsSSRImport", compilation => {
      compilation.mainTemplate.hooks.requireEnsure.tap(
        "NextJsSSRImport",
        (code, chunk) => {
          // This is the block that fixes https://github.com/vercel/next.js/issues/22581
          if (!chunk.name) {
            return;
          }

          // Update to load chunks from our custom chunks directory
          const outputPath = resolve("/");
          const pagePath = join("/", dirname(chunk.name));
          const relativePathToBaseDir = relative(pagePath, outputPath);
          // Make sure even in windows, the path looks like in unix
          // Node.js require system will convert it accordingly
          const relativePathToBaseDirNormalized = relativePathToBaseDir.replace(
            /\\/g,
            "/"
          );
          return code
            .replace(
              'require("./"',
              `require("${relativePathToBaseDirNormalized}/"`
            )
            .replace(
              "readFile(join(__dirname",
              `readFile(join(__dirname, "${relativePathToBaseDirNormalized}"`
            );
        }
      );
    });
  };
}

module.exports = {
  reactStrictMode: true,
  headers: async () => [
    "/",
    "/_next/static/chunks/:slug",
  ].map(source => ({
    source,
    headers: [
      {
        key: 'Cross-Origin-Opener-Policy',
        value: 'same-origin',
      },
      {
        key: 'Cross-Origin-Embedder-Policy',
        value: 'require-corp',
      },
    ],
  })),
  webpack: (config, { buildId, dev, isServer, defaultLoaders, webpack }) => {
    const ssrPlugin = config.plugins.find(plugin => plugin instanceof SSRPlugin);
    if (ssrPlugin) patchSsrPlugin(ssrPlugin);

    config.module.rules.find(k => k.oneOf !== undefined).oneOf.unshift(
      {
        test: /\.wasm$/,
        type: "asset/resource",
        generator: {
          filename: 'static/chunks/[path][name].[hash][ext]'
        },
      }
    );

    config.output.chunkFilename = isServer
      ? `${dev ? "[name]" : "[name].[fullhash]"}.js`
      : `static/chunks/${dev ? "[name]" : "[name].[fullhash]"}.js`;
      
    return config;
  },
}

That was hard... But I'm happy that it finally works now.

@rmadsen
Copy link

rmadsen commented Oct 18, 2021

+1 to @tomlagier 's patch (after adding a return config line of course). In my case, the webpack/runtime/compat error came from a import Prism from 'prismjs/components/prism-core.js' in a dependency used by a webworker.

tom-sherman pushed a commit to tom-sherman/next.js that referenced this issue Dec 13, 2021
This skips 'require' normalization for chunks without a name, which was throwing errors when used with WASM imports. See vercel#22581.
@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants