parcel-bundler / parcel

The zero configuration build tool for the web. πŸ“¦πŸš€
https://parceljs.org
MIT License
43.38k stars 2.27k forks source link

Compatibility issues with node workers / `comlink` #4731

Open lgarron opened 4 years ago

lgarron commented 4 years ago

See https://github.com/lgarron/parcel-bug-worker-node-external-modules for full details/repro.

πŸ› bug report

Using the comlink library with a node target in Parcel 2 fails to produce working code. :-(

πŸŽ› Configuration (.babelrc, package.json, cli command)

This issue crops up when adapting this example with fairly straightforward settings:

package.json:

{
  "main": "dist/index.js",
  "targets": {
    "main": {
      "context": "node"
    },
    ...
  }

CLI command:

// --no-minify helps with debugging, but does not cause/affect the issues in questions.
npx parcel build --no-minify src/main.ts
node dist/index.js

πŸ€” Expected Behavior

The resulting code runs in node with a worker, and prints the correct output (4).

😯 Current Behavior

There are two main issues.

Without "includeNodeModules": true in package.json:

> cd issue1
> npm install
> npx parcel build src/main.ts

🚨 Build failed.
@parcel/packager-js: External modules are not supported when building for browser
/Users/lgarron/parcel-bug-worker-node-external-modules/issue1/src/worker.ts:1:1
> 1 | import * as Comlink from "comlink";
>   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  2 | import nodeEndpoint from "comlink/dist/esm/node-adapter";
  3 | import { parentPort } from "worker_threads";

With "includeNodeModules": true in package.json:

> cd issue2
> npm install
> npx parcel build src/main.ts
> node dist/index.js

(node:17465) UnhandledPromiseRejectionWarning: Error: Origin not found
    at Object.L [as getOrigin] (/Users/lgarron/parcel-bug-worker-node-external-modules/issue2/dist/index.js:1:3467)
    at k (/Users/lgarron/parcel-bug-worker-node-external-modules/issue2/dist/index.js:1:3708)
    at P (/Users/lgarron/parcel-bug-worker-node-external-modules/issue2/dist/index.js:1:3873)
    at /Users/lgarron/parcel-bug-worker-node-external-modules/issue2/dist/index.js:1:10995
    at s (/Users/lgarron/parcel-bug-worker-node-external-modules/issue2/dist/index.js:1:4954)
    at Generator._invoke (/Users/lgarron/parcel-bug-worker-node-external-modules/issue2/dist/index.js:1:4707)
    at Generator.forEach.t.<computed> [as next] (/Users/lgarron/parcel-bug-worker-node-external-modules/issue2/dist/index.js:1:5311)
    at G (/Users/lgarron/parcel-bug-worker-node-external-modules/issue2/dist/index.js:1:10554)
    at i (/Users/lgarron/parcel-bug-worker-node-external-modules/issue2/dist/index.js:1:10757)
    at /Users/lgarron/parcel-bug-worker-node-external-modules/issue2/dist/index.js:1:10816
(Use `node --trace-warnings ...` to show where the warning was created)
(node:17465) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:17465) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

πŸ’ Possible Solution

Issue 1: It seems that Parcel is running into an issue with external imports inside a worker, and mistaking this for an issue with browser builds. I'm not sure what's causing this.

Issue 2: The generated code contains this:

function $cfe9adb4ece96cf6af2236c88b055$var$getBundleURL() {
  try {
    throw new Error();
  } catch (err) {
    var matches = ('' + err.stack).match(/(https?|file|ftp):\/\/[^)\n]+/g);

    if (matches) {
      return $cfe9adb4ece96cf6af2236c88b055$var$getBaseURL(matches[0]);
    }
  }

  return '/';
}

// ...

function (relativePath) {
  var workerUrl = $e107214d8467b08318ea26a0cebe9$var$bundleUrl.getBundleURL() + relativePath;

  if ($e107214d8467b08318ea26a0cebe9$var$bundleUrl.getOrigin(workerUrl) === self.location.origin) {
    // If the worker bundle's url is on the same origin as the document,
    // use the worker bundle's own url.
    return workerUrl;
  } else {
    // Otherwise, create a blob URL which loads the worker bundle with `importScripts`.
    return URL.createObjectURL(new Blob(['importScripts(' + JSON.stringify(workerUrl) + ');']));
  }
};

The bundle URL is calculated as /, which results in a worker URL of /worker.09ef4db6.js. I haven't found a simple fix for this, but the correct worker URL from inside that file would be ./worker.09ef4db6.js. However, note that hacking the code above so that workerUrl has this value doesn't work.

(EDIT: Note that in most environments, node will need to use something like __dirname rather than a relative path.)

πŸ”¦ Context

I'm working on several libraries that are meant work on both node and the browser. Due to assumptions by various bundlers (including Parcel) when consuming these libraries, it's important that this works in a single build.

To replicate the issue within context:

git clone https://github.com/cubing/jsss
cd jsss
git checkout main
npm install
npx parcel build --target main src/targets/main.ts

Note that the project still contains separate targets for node/module/browser, because I'm also working around multiple other issues at the same time (e.g. Parcel's cache seems to assume that any entry file in a project only has one target, and it's impossible to build types with the default target without building more targets than intended).

πŸ’» Code Sample

https://github.com/cubing/jsss/blob/main/src/random-scramble.ts

🌍 Your Environment

Software Version(s)
Parcel 2.0.0-nightly.270
Node v14.3.0
npm/Yarn npm 6.14.4
Operating System macOS 10.15.5

I'd appreciate any help on either how to fix these issues, or how to configure a project with workers "properly". Parcel has otherwise been pretty zero-config for me, and given that the original code works directly in node I'd really like to be able to build it with Parcel without any hacks.

mischnic commented 4 years ago

The lgarron/parcel-bug-worker-node-external-modules repo doesn't exist

mischnic commented 4 years ago

Issue 1: It seems that Parcel is running into an issue with external imports inside a worker, and mistaking this for an issue with browser builds. I'm not sure what's causing this.

We almost certainly need a node-worker context here:

https://github.com/parcel-bundler/parcel/blob/e95fbaeac705c170719f771580e502c1f7f5865a/packages/core/types/index.js#L89-L95

Then the new Worker handling would have to set the context accordingly (if the current context is node or browser): https://github.com/parcel-bundler/parcel/blob/e95fbaeac705c170719f771580e502c1f7f5865a/packages/transformers/js/src/visitors/dependencies.js#L164

And adding the new context to these list as appropriate: https://github.com/parcel-bundler/parcel/blob/e95fbaeac705c170719f771580e502c1f7f5865a/packages/core/core/src/Environment.js#L30-L78 and https://github.com/parcel-bundler/parcel/blob/e95fbaeac705c170719f771580e502c1f7f5865a/packages/core/core/src/public/Environment.js#L15-L24

Issue 2: The generated code contains this:

After 1), JSRuntime might needs to be adapted to generate the correct code for the node worker.

lgarron commented 4 years ago

The lgarron/parcel-bug-worker-node-external-modules repo doesn't exist

Apologies, it seems I accidentally created it as private (!). It's public now.

Do you know if those fixes can allow the same code to be run as a worker to be run in node as well as in the browser? Especially with a library link comlink, it's easy to write something that works in both.

lgarron commented 4 years ago

Is there a workaround I could use for now? I'm unable to move forward on an important project because Parcel fails to build it. (This is still the case for the latest nightly.)

lgarron commented 4 years ago

Is there a workaround I could use for now? I'm unable to move forward on an important project because Parcel fails to build it. (This is still the case for the latest nightly.)

I'll also note that this works just fine in the browser using parcel serve during development! It's just parcel build that errors.

egasimus commented 3 years ago

@lgarron I've submitted a PR for a related issue. Do you think it'll help in your case?

lgarron commented 3 years ago

@lgarron I've submitted a PR for a related issue. Do you think it'll help in your case?

I can't quite tell. What happens if you try to run https://github.com/lgarron/parcel-bug-worker-node-external-modules ?

egasimus commented 3 years ago

@lgarron It was not easy for me to tell, either :) I tested issue2 earlier and it looks like my fix only works for browsers (because of self.location.origin.)

Apparently I don't know how to properly vendor a workspace, because I tried to add my patched fork of Parcel to my project (which is also a workspace), and, well, it's confusing. I ended up just editing node_modules in issue2 until I managed to get the worker to complain about a missing postMessage instead of the host complaining about a missing Origin, so, uh, progress?

To achieve that:

And I guess __dirname wouldn't be available in CJS-less environments, either? I've only been looking at Parcel's source since yesterday so I don't know how the origin fits into the grand scheme but it looks like we need a platform-independent way of fetching it. I wonder if there's anything more reliable than parsing the stack trace.

Hope any of this helps...

lgarron commented 3 years ago

Looks like this miiiight already be working, at least with a worker in a library?

You can test with npm install cubing and:

import { randomScrambleForEvent } from "cubing/scramble";

randomScrambleForEvent("333").then((s) => s.log());

The console should show:

Successful relative URL worker instantiation.
Screen Shot 2021-08-19 at 18 49 14

This should show if it doesn't work:

WARNING: Could not instantiate and communicate with a relative URL worker. This means that your app may have issues with `cubing/solve` in the future.

But I think I had more issues in dev mode, not with already-built libraries.

zakjan commented 2 years ago

Is importing 3rd-party libraries into WebWorkers going to be supported? I tried both with and without includeNodeModules: true, same result.

  "targets": {
    "default": {
      "includeNodeModules": true
    }
  }

Error:

@parcel/packager-js: External modules are not supported when building for browser

There is comlink + a few other computationally expensive libraries imported in the worker code.

Until it is supported with Parcel, I stay with Rollup and rollup-plugin-worker-factory.

Dingzhaocheng commented 9 months ago

ping