parcel-bundler / parcel

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

Parcel breaks `import.meta.url` in worker entry files #7623

Open lgarron opened 2 years ago

lgarron commented 2 years ago

šŸ› bug report

Parcel breaks trampoline code for worker instantiation:

// index.js
const workerURL = (await import("./worker.js")).WORKER_ENTRY_FILE_URL;
const blob = new Blob([`import "${workerURL}";`], { type: "text/javascript" });
new Worker(URL.createObjectURL(blob), { type: "module" });

// worker.js
export const WORKER_ENTRY_FILE_URL = import.meta.url;

šŸ¤” Expected Behavior

The worker is instantiated properly, as it does when the source code is served either directly or with other tools:

Command URL
npx serve (or any static file server) http://localhost:3000/src/
npx vite http://localhost:3000/src/
npx wmr http://localhost:8080/src/
npx esbuild src/index.js --format=esm --bundle --splitting --servedir=src http://localhost:8000/src/

šŸ˜Æ Current Behavior

Parcel errors with:

Not allowed to load local resource: file:///src/worker.js

šŸ’ Possible Solution

There should be a way to preserve the semantics of import.meta.url.

At the very least, it would be sufficient in our case if import.meta.url is left untransformed when it is assigned to an export.

šŸ”¦ Context

We maintain a library that:

1) is used in node and on the web (in particular, is available from a CDN), and 2) must use web workers for acceptable end-user experience, and 3) is published and consumed by bundlers through npm.

Unfortunately, instantiating workers in a portable way is challenging. The trampoline approach is the closest to a universal approach.

šŸ’» Code Sample

See https://github.com/lgarron/parcel-import-meta-url-bug for a runnable repro:

git clone https://github.com/lgarron/parcel-import-meta-url-bug && cd parcel-import-meta-url-bug
npx parcel src/index.html

šŸŒ Your Environment

Software Version(s)
Parcel 2.2.1
Node v17.4.0
npm/Yarn npm 8.3.1
Operating System macOS 12.2
devongovett commented 2 years ago

import.meta.url is rewritten to be relative to the project root to avoid exposing absolute paths on your build machine. See https://parceljs.org/languages/javascript/#import.meta

In addition, writing the code this way means Parcel is not aware that it will run in a worker environment, which is important for things like runtime loading. Writing the code in this form is how Parcel can determine that:

let worker = new Worker(new URL('worker.js', import.meta.url));

However, it looks like you are trying to work around the same origin restriction with workers by using a blob url. Parcel actually does this automatically by default. Here's an example. If you inspect the code, you can see that if the worker origin is different from the page origin, we apply this exact workaround. Hopefully this solves your issue.

lgarron commented 2 years ago

Hopefully this solves your issue.

Unfortunately, it can't. :-/

  1. We need our code to include the blob workaround ourselves, to make sure our code works in other bundlers.
  2. We can't use new URL('worker.js', import.meta.url) since not all bundlers don't preserve the semantics (at least, not as widely as they preserve dynamic import entries). In particular, we can't do it in our own project without plugins or hacks.

It looks like eval("import.meta.url") actually does preserve the semantics, but obviously including eval in our library code is very undesirable. Do you possibly have any other workarounds or would be open to introducing any that are compatible and preserved by other bundlers?

devongovett commented 2 years ago

I would argue that passing import.meta.url through as is does not preserve semantics though. It should work the same way as it would natively, without bundling. For that, import.meta.url needs to refer to the source file, not to the compiled bundle. Otherwise, if multiple modules used import.meta.url to refer to themselves, the result would be that they all refer to the same url rather than different ones, breaking the original semantics of the code.

Parcel matches webpack's behavior here. Webpack also compiles import.meta.url to a file:// url pointing to the original source file path.

lgarron commented 2 years ago

It should work the same way as it would natively, without bundling.

I certainly agree with this sentence, so I guess it comes down to which semantics should be preserved. https://github.com/lgarron/parcel-import-meta-url-bug shows that the untransformed code successfully instantiates the worker, using native semantics... and that Parcel breaks the native functionality (unless we add the eval wrapper). I don't use Webpack, but every other bundler I've tested keeps our code working[^1].

Given our tradeoffs, we currently have no choice but to instantiate the worker this way ā€” new URL('worker.js', import.meta.url) is simply not compatible with enough bundlers. :-/

If Parcel can't address this, should we recommend that folks shouldn't use Parcel to build applications using our library? That would make me sad, but it helps me know how to handle this situation going forward.

[^1]: Except Snowpack, who explicitly says they don't support workers.

devongovett commented 2 years ago
const workerURL = (await import("./worker.js")).WORKER_ENTRY_FILE_URL;

This is a strange approach. It will result in the module being executed twice: once in the main thread, and again in the worker.

Why not do something like this?

const workerURL = new URL('worker.js', import.meta.url);

This works natively and in most bundlers (e.g. Webpack, Vite, Parcel).

the untransformed code successfully instantiates the worker, using native semantics

Yes, because it isn't bundled. Once bundled, the source module doesn't exist anymore, it's embedded in the bundle, so there's no individual URL to refer to. Referring to the entire bundle is incorrect. For example:

a.js

import './b.js';
console.log(import.meta.url);

b.js

console.log(import.meta.url);

These should log two different results, but if bundled and untransformed, they would log the same result. That would break many things.

lgarron commented 2 years ago

Also, apologies if I sound really cranky about this. I'm a little annoyed at myself for not noticing this sooner. I thought that this worked in all bundlers, given that it was passing our test against Parcel in CI ā€” but unfortunately it turns out that this was because of our super blunt workaround of serializing an entire worker source to a 1MB string.

This situation is not Parcel's fault, and Parcel in fact has done more than most bundlers to make workers less painful.

But I'm determined to remove all workarounds as soon as module workers are available in Firefox (which looks like it will hopefully happen soon), and this is the closest to a universal approach that seems possible. I don't want to leave Parcel compatibility behind. šŸ˜”

lgarron commented 2 years ago

Why not do something like this?

const workerURL = new URL('worker.js', import.meta.url);

This works natively and in most bundlers.

This does not work in esbuild (as referenced above), and unfortunately that immediately removes it as a possibility for us ā€” even if we try switching bundlers within our project, it means any project using esbuilid for bundling will break.

devongovett commented 2 years ago

Hmm, I think esbuild is the odd one out here then. As I linked above in an edit, most other bundlers support this syntax and it is supported natively as well. I would hope that esbuild would add support for it as well.

lgarron commented 2 years ago

As I linked above in an edit, most other bundlers support this syntax and it is supported natively as well. I would hope that esbuild would add support for it as well.

That may be ā€” I've been trying to get esbuild to consider it. But I don't have the luxury of breaking esbuild compatibility until that happens.

devongovett commented 2 years ago

Anyway, I think the approach proposed in the original issue will also have problems in webpack, which seems like a pretty big target to exclude for a library too. https://webpack.js.org/api/module-variables/#importmetaurl

lgarron commented 2 years ago

Anyway, I think the approach proposed in the original issue will also have problems in webpack, which seems like a pretty big target to exclude for a library too. https://webpack.js.org/api/module-variables/#importmetaurl

Then I consider this a serious compatibility bug in Webpack as well. šŸ¤·

In any case, I have often used ā€” and still recommend ā€” Parcel. I'm not nearly as concerned with Webpack compat.

devongovett commented 2 years ago

I'm not sure what else to tell you, sorry! esbuild's behavior is incorrect here. You should open an issue with them to support the URL constructor (which can be used for web workers, but also other kinds of dependencies like images). Parcel and webpack implement import.meta.url and new URL correctly, as I've tried to explain. I don't think this is a bug in Parcel.

lgarron commented 2 years ago

I don't think this is a bug in Parcel.

Sounds like we just fundamentally disagree on whether Parcel should break this code. šŸ¤· (I certainly make a distinction between "unsupported functionality" ā€” like in esbuild and snowpack ā€” vs. "changes code on purpose".)

I've left another comment on the esbuild issue, and will see if I can get in contact with the maintainer. Let's hope we can end up in a situation where libraries don't have to make an impossible tradeoff. šŸ¤ž

devongovett commented 2 years ago

whether Parcel should break this code

I know you're frustrated, but please try to understand how it works. I feel like I've tried to explain it a few times now. Think about it: where do your source files exist? How do you refer to them? From Parcel's perspective, while building your code, your files are on the file system. There's no URL to link to if a module is bundled into the same file with other modules.

Let me extend your example a little.

// index.js
const workerURL = (await import("./worker.js")).WORKER_ENTRY_FILE_URL;
const blob = new Blob([`import "${workerURL}";`], { type: "text/javascript" });
new Worker(URL.createObjectURL(blob), { type: "module" });

// worker.js
export {LIBRARY_URL as WORKER_ENTRY_FILE_URL} from "./library.js";

// library.js
export const LIBRARY_URL = import.meta.url;

If you load this in a browser, workerURL will be the URL of library.js due to the re-export. But if you run it through a bundler first, without transforming import.meta.url, now it refers to the entry of the bundle, e.g. worker.js, because both worker.js and library.js are in the same file. There's no way to uniquely access library.js at that point. This would be broken in esbuild.

Therefore, import.meta.url refers to the source file, as it existed on the file system in your project. This correctly preserves module semantics, even when bundled. You may not like that it is a file:// url rather than an http:// url, but that is required for bundling to work properly. Think of import.meta.url as referring to a source module, not a bundled file.

lgarron commented 2 years ago

Therefore, import.meta.url refers to the source file, as it existed on the file system in your project. This correctly preserves module semantics, even when bundled. You may not like that it is a file:// url rather than an http:// url, but that is required for bundling to work properly. Think of import.meta.url as referring to a source module, not a bundled file.

I think you make a good argument that this makes sense for applications that rely in import.meta.url to be a unique identifier for a source file (at least, without putting each such file into a separate output file).

However, in practice bundlers all seem to preserve dynamic imports for code splitting, and eval(import.meta.url) does work in Parcel as expected for this use case.

afogel commented 2 years ago

@devongovett I opened an issue that may be related (#7643), but I guess I'm still a bit confused (apologies for my n00bliness!). Perhaps I can restate the issue here in simple (and perhaps somewhat reductionist) terms and try to identify how it may be similar/different from my issue?

It seems like lgarron wants the import.meta.url to resolve to a server which will serve up the worker file as needed from an abritrary origin (cdn, or otherwise). This seems to interfere with how parcel analyzes source directories to be served in a particular bundle, which is why it reverts to the file:// origin. Hopefully I haven't butchered this exchange too much -- I'm not too familiar with the internals of how bundlers work (though I'm very grateful for the work they and you do!)

I'm wondering whether this is due to the fact that the file lgarron is serving is a worker file and therefore, a .js file. Given that, I would imagine that parcel might have difficulty differentiating which files should be incorporated into the initial bundle and which should be served later. If that holds, wouldn't the bundler handle filetypes like videos differently, though?

I would imagine that images/videos aren't included in the bundled chunks, and therefore would be serve-able after the fact from the server?

One final question -- in development, given that import.meta.url was rewritten to be relative to the project root to avoid exposing absolute paths on the build machine, doesn't that mean that all file:// URLs will be broken links on my build machine? Or are there circumstances/ways to get them to still serve/find the assets that I haven't encountered yet?

If they will always be broken in development, would it be worthwhile adding a feature/flag that would enable them to be served from build machine using either an absolute path in development or from localhost?

Thanks in advance for your patience, time, and consideration!

afogel commented 2 years ago

@devongovett I just wanted to circle back and ask about this, as I'm currently having to do a hacky work-around to get a similar-sounding issue resolved in my app. I was hoping whether you might be able to help me better understand about import.meta.url and help me get insight into what I might be encountering? Thanks!

afogel commented 2 years ago

@devongovett ping?

afogel commented 2 years ago

@devongovett ping again?

h4de5 commented 2 years ago

I have read throughout this conversation and I do understand the maintainers and the users point of view somehow.

I just want to share some insights from using webpack for a similar issue.

we wanted to develop the worker script in typescript rather than in javascript. so webpack/tsc needs to transpile the typescript into javascript first and than provide a link to that javascript file, that works within a deployed environment - so no file:/// links. also we out-sourced the shared worker into it's own library, so multiple applications can use it, instead of having the same shared worker file duplicated.

instance SharedWorker:

return new SharedWorker(
  new URL(
    '@my/api/shared-worker',
    import.meta.url
  ) /* webpackChunkName: 'shared-worker' */,
  {
    type: 'module',
    name: 'My Shared Worker',
  }
);

the interessting part here is, that the combination new SharedWorker(new URL('xxx', import.meta.url)) must be written like this. it is not possible put any of those terms into a variable. otherwise webpack will not produce a working worker, or will even prevent it from compiling..

it seems webpack handles to the combination of (Shared)Worker and URL specially to just fullfill the usecase.

https://webpack.js.org/guides/web-workers/

more from the example - in case someone stumbles across this.

tsconfig.ts

...
  "@my/api/shared-worker": [
    "libs/api/worker/shared-worker.ts"
  ],
...

api module`s index.ts

....
// May not be exported because otherwise the worker will not be executed in the web worker context.
// But must be imported so that the file is compiled.
import * as SharedWorker from '@my/api/shared-worker';

shared-worker.ts

imports ...

export default self.onconnect = (event: MessageEvent<any>) => {
  const [port] = event.ports;
  start(port);
};

if ('DedicatedWorkerGlobalScope' in self) start(self as unknown as MessagePort);

function start(port: MessagePort) {
  // do worker stuff
  port.postMessage({ data: 'data' })
  port.onmessage = (e: MessageEvent) => {
      messageHandler(e, port, subscriptions, dpConnectSubs);
  };
}
afogel commented 1 year ago

though perhaps old, this is still relevant

lgarron commented 1 year ago

though perhaps old, this is still relevant

Yeah, it's been over a year and the choice continues to be:

With both approaches it's "obvious" how to maintain correctness with standard code splitting, so it's a matter of the ecosystem settling on a cross-compatible solution.

For now, we've given up on Parcel compatibility, given that this issue is stalled.

lgarron commented 1 year ago

This is still a breaking issue with Parcel for us, at least unless/until https://github.com/parcel-bundler/parcel/issues/8924 is implemented.

lgarron commented 6 months ago

As far as I know, this is still relevant.

lgarron commented 1 week ago

I'm going to unsubscribe from this issue, as I don't see any hope of it being addressed and I have given up on writing Parcel-compatible code as a result.

As noted in the first post, all other major bundlers properly maintain the path resolution semantics in the example, and have continued to do so for over 2.5 years. I would still highly recommend that Parcel changes its design decisions to match them for this kind of code.

I've made sure that https://github.com/lgarron/parcel-import-meta-url-bug still serves as a reproduction case, in case you still wish to address this.