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

Convention for libraries to provide "workers" compatible bundle. #84

Closed
nicksrandall opened this issue Dec 9, 2021 · 13 comments
Closed
Labels
enhancement New feature or request

Comments

@nicksrandall
Copy link
Contributor

An issue that I regularly run up against when using Cloudflare Workers is that a library that I want to use provides both "node" and "browser" compatible bundles but neither of them work in the Cloudflare Workers environment because the bundles rely on node apis or DOM apis that are not available in Workers.

I think it would be great to establish some kind of convention that library authors can use to provide a third bundle that specifically lists itself as compatible with a "workers" style environment. While there would certainly be an advantage to marking a package specifically as "Cloudflare Workers" compatible, I think it would be cool if we could extend this to any DOM-less javascript environment like "service workers" or traditional "workers" in the browser. I'm not sure that makes sense but I figured would list it here as a possible area of exploration.

A great example of this issue can be found with the @emotion family of packages on NPM. Their package.json files look something like:

{
  "name": "@emotion/react",
  "version": "11.7.0",
  "main": "dist/emotion-react.cjs.js",
  "module": "dist/emotion-react.esm.js",
  "browser": {
    "./dist/emotion-react.cjs.js": "./dist/emotion-react.browser.cjs.js",
    "./dist/emotion-react.esm.js": "./dist/emotion-react.browser.esm.js"
  },
}

They are obviously trying to be good citizens by providing both node and browser compatible bundles (in both cjs and 'mjs` formats) but neither of those are compatible with workers because the node version uses node apis like "stream" that do not work in workers and the browser bundle uses some DOM apis. Here is a minimal reproduction repo of this issue with wrangler2 and emotion: https://github.com/nicksrandall/emotion-workers-example/tree/main

NOTE: This issue is a result of this twitter conversation: https://twitter.com/NickSRandall/status/1468991859267497988

@nicksrandall
Copy link
Contributor Author

Here is some related discussion in emotion package: emotion-js/emotion#2554
Here is some related discussion in precontruct package: preconstruct/preconstruct#431

Context: Emotion uses preconstruct to generate bundles so a fix to emotion would require a fix to preconstruct.

@Andarist
Copy link
Contributor

Andarist commented Dec 9, 2021

I think that the answer, for the most part, is to make use of package.json#exports.worker but yes - I believe this should be clearly stated somewhere here. For instance, at the moment this condition is not utilized by Cloudflare's Rollup starter - I've prepared a fix for this here: cloudflare/durable-objects-rollup-esm#14 . IIRC the webpack starter take this fully into account but the Cloudlfare docs suggest that it does - or smth like that, I would have to refresh my memory on this part to be sure.

@threepointone
Copy link
Contributor

We could very easily use an exports field, and it'll be easier in wrangler2 assuming folks outsource module resolution etc to us. Just want to be certain what field we settle on. 'worker' usually implies a web worker, but of course we're not fully that either. anyway, I'll put a PR for this early next week.

@Andarist
Copy link
Contributor

@threepointone is this part of the docs up to date? is wrangler2 supporting webpack anyhow - I've glimpsed to the code and it seems that the dev and production builds are done using esbuild, right?

Either way - I've checked the esbuild's code and docs and luckily it already supports conditions: https://esbuild.github.io/api/#conditions

Note that you could also make use of browser, worker, production and development conditions. Since this is what other bundlers (like webpack) might do with target: 'webworker'

We could very easily use an exports field, and it'll be easier in wrangler2 assuming folks outsource module resolution etc to us.

If they don't - well, it's up to them to configure stuff on their own. I would only make sure that your existing boilerplates are configured in the same way as wrangler2 is (when it comes to this resolution stuff).

Just want to be certain what field we settle on. 'worker' usually implies a web worker, but of course we're not fully that either.

That is true - I wonder though, is there any notable difference for people? From what I understand you are taking the high road and you are staying away from implementing any custom APIs etc and you are all about web standards. Is there anything that webworkers can do that is not supported on Cloudflare (and vice-versa)? I'm just a little bit worried about introducing too many conditions to the world - this stuff is not that easy to handle for package authors as JS can run in so many environments.

@threepointone
Copy link
Contributor

None of the docs reflect wrangler2 just yet. Maybe webworker should be enough. We're close enough to it that it shouldn't matter (except when it actually does, haha).

@Andarist
Copy link
Contributor

Maybe webworker should be enough.

Just a word of caution - IIRC the condition is 'worker' while the target in webpack is 'webworker'.

it shouldn't matter (except when it actually does, haha).

Put that on my tombstone.

@threepointone
Copy link
Contributor

ugggh. we have separate work underway to eject the webpack stuff from wrangler 1 projects. I'll make sure to limit the splash there.

@nicksrandall
Copy link
Contributor Author

FWIW, esbuild supports custom package exports conditions so you could support this today by adding conditions: ['worker'] to wrangler's esbuild config.

IIRC, the list is in order of preference so you could add something even more specific to the front of the list that could fix a conflict between worker (ie webworker) and Cloudflare workers.

Maybe something like this?:

{
  conditions: ['cloudflare-worker', 'worker', 'browser', 'module', 'default']
}

See: https://esbuild.github.io/api/#conditions

@xortive
Copy link
Contributor

xortive commented Dec 10, 2021

webworker is different enough from cloudflare workers that we shouldn't overload it -- we have several differences from the service-worker API, and module workers aren't service workers at all!

Is there anything that webworkers can do that is not supported on Cloudflare

A big one is threads, so tooling that assumes the "webworker" target allows their use tends to break. The vast majority of things don't do this, but IIRC emscripten surprised me by generating something that tried to use them when you enabled its webworker target.

I'd be in favor of cloudflare-worker

@nicksrandall
Copy link
Contributor Author

@xortive I understand (and agree) that web-worker compatibility does not guarantee cloudflare worker compatibility. If a library wanted to explicitly support cloudflare workers then it would make sense to use the explicit conditional, whatever we decide that to be. That said, if that explicit conditional is not present but a "worker" conditional is, I think it is highly likely that we would want to use that since it is the closest environment to cloudflare.

I guess what I'm saying is that I think we could make a strong case to support both, in order of preference.

@nicksrandall
Copy link
Contributor Author

@threepointone I'm happy to submit PRs for the esbuild stuff if you think that this is worth pursuing.

@Andarist
Copy link
Contributor

That said, if that explicit conditional is not present but a "worker" conditional is, I think it is highly likely that we would want to use that since it is the closest environment to cloudflare.

I agree on that one. If a more specific condition has to be introduced in the future (or even now) - it can be done. But "worker" should be used as a fallback, and even maybe a "browser" (with lower priority than "worker", ofc). At least this is how it works in webpack for target: 'webworker' and this has been "promoted" in the Cloudflare docs for Wrangle v1. So at least nothing would be that surprising about this - unless you think that this is an entirely bad idea.

@threepointone threepointone added the enhancement New feature or request label Dec 12, 2021
@threepointone
Copy link
Contributor

I'm going to mark this issue as closed. Of course libraries will have to provide bundles with worker exports, and we'll have to eventually come up with something specific for cloudflare, but I'm grateful for @nicksrandall and @Andarist for the PR we just landed in #93. Thanks thanks thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants