LoadingManager: Lazily instantiate abort controller.

[NuroDev]

Description

This is a fix for a very niche edge case, but wanted to at least propose a fix for it.

The problem

For context, I work for @ronin-co and we are building blade, a React framework for building instant web apps. With this we have found when deploying to Cloudflare Workers, which is powered by workerd, their API throws an error if a Blade project tries to use Three.js:

Uncaught Error: Disallowed operation called within global scope. Asynchronous I/O (ex: fetch() or connect()), setting a timeout, and generating random values are not allowed within global scope. To fix this error, perform this operation within a handler. https://developers.cloudflare.com/workers/runtime-apis/handlers/
  at public/client/chunk.3154980606.js:37678:27 in LoadingManager
  at public/client/chunk.3154980606.js:37828:48 in cdn:https://unpkg.com/three
  at public/client/chunk.3154980606.js:10:51
  at public/client/chunk.3154980606.js:48683:46

After a bit of investigating I found this issue outlining that workerd does not currently support creating a new abort controller instance in global scope, which theLoadingManager loader seems to rely on to construct a new instance.

The solution

I found this PR which defers calling new AbortController() on class construction, and instead uses a getter to manage it. Testing this on our Blade project that uses Three.js it seems to fix the issue.

With this I have added some basic unit tests to validate this logic, however I am not deeply familiar with this code base so there could be something I am missing here.

[github-actions]

📦 Bundle size

Full ESM build, minified and gzipped.

	Before	After	Diff
WebGL	350.11 84.81	350.11 84.81	+0 B +0 B
WebGPU	604.76 169.55	604.76 169.55	+0 B +0 B
WebGPU Nodes	603.37 169.31	603.37 169.31	+0 B +0 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

	Before	After	Diff
WebGL	481.77 119.56	481.77 119.56	+0 B +0 B
WebGPU	674.14 185.02	674.14 185.02	+0 B +0 B
WebGPU Nodes	616.13 168.25	616.13 168.25	+0 B +0 B

[Mugen87]

I'm not sure it's appropriate to add a fix for a bug in another library. Have you considered to bump cloudflare/workerd#3657 and ask for an update?

TBH, I'm much not in favor of this fix since it feels a bit unintuitive.

[mrdoob]

Could we accept this change temporarily until cloudflare/workerd#3657 is fixed?

[Mugen87]

Because @NuroDev didn't do it, I have bumped the issue at workerd via cloudflare/workerd#3657 (comment). Let's see how the maintainers respond.

[leo]

Could we accept this change temporarily until cloudflare/workerd#3657 is fixed?

In my opinion, that would be a very good idea.

Because while jasnell calls it a bug on the linked issue, Cloudflare Workers explicitly introduced a bunch of similar limitations, such as preventing the following outside the request handler:

• Asynchronous I/O (e.g. fetch)
• Timers (e.g. setTimeout and setInterval)
• Cryptographic randomness (e.g. crypto.randomUUID and crypto.getRandomValues)
• etc.

So while I do believe that it might get fixed (AbortController specifically), I don't think it will get fixed anytime soon, because several of those are inherent limitations of CF's compute model (one app per thread instead of per process) and the respective security requirements.

At the moment, Three.js does not work at all on Cloudflare Workers. Libraries like OpenNext (used to deploy Next.js to Cloudflare Workers) use a sledge hammer solution to avoid such constraints by importing the entire application inside the request hander, which obviously slows down the worker since the code is now evaluated when the first request comes in, rather than at bootup time. That's why Three.js currently "works" when used in e.g. Next.js on CF. It's not a reasonable solution for production applications.

[Mugen87]

Um, would you at least add a comment in LoadingManager that clarifies this change is not intended as a permanent solution? It should be removed as soon as cloudflare/workerd#3657 is fixed.

I don't think it will get fixed anytime soon,

@jasnell Is that true? I was hoping we could avoid adding such code changes to three.js, tbh.

[NuroDev]

Sure, that makes total sense @Mugen87. I've added a TODO to the abort controller getter as a note for now.
I've also subscribed to that workerd issue so once it has been fixed I'll make a quick PR to revert this change.

[leo]

@Mugen87 Thanks a lot for the feedback! Do you think the PR could be merged now? Otherwise, please feel free to let us know what else should be changed.

[Mugen87]

I have approved the change. I was hoping to get some feedback from the Cloudflare team in time but I agree it's better to merge before waiting too long.

[leo]

Incredible, thank you so much! 🙏