Animation: Fix ReferenceError in non-broswer environment

[chirsz-ever]

Node.js and Deno do not support self.

[github-actions]

📦 Bundle size

Full ESM build, minified and gzipped.

	Before	After	Diff
WebGL	336.36 78.34	336.36 78.34	+0 B +0 B
WebGPU	533.31 148.2	533.34 148.21	+30 B +12 B
WebGPU Nodes	532.78 148.1	532.81 148.11	+30 B +12 B

🌳 Bundle size after tree-shaking

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

	Before	After	Diff
WebGL	465.37 112.21	465.37 112.21	+0 B +0 B
WebGPU	604.9 164.17	604.93 164.18	+30 B +12 B
WebGPU Nodes	559.88 153.6	559.91 153.61	+30 B +12 B

[gkjohnson]

Why not just use globalThis which is the standard for JS? Then build tools can polyfill this based on build target.

[chirsz-ever]

I do not find globalThis used anywhere else in this repository, so I assume it’s some kind of convention.

@mrdoob do you think using globalThis is OK?

[Mugen87]

The main reason is that we've decided to stick to ES2018 for compatibility reasons. globalThis is ES2020.

However, if the usage of globalThis does not affect build tools like WebPack 4, I'm okay with using it.

[Mugen87]

Why not just doing this similar to WebGLRenderer:

this._context = typeof self !== 'undefined' ? self : null;

The code is a bit different but the outcome is the same (context evaluates to null):

three.js/src/renderers/WebGLRenderer.js

Line 1494 in d3ca4be

if ( typeof self !== 'undefined' ) animation.setContext( self );

[chirsz-ever]

Why not just doing this similar to WebGLRenderer:

this._context = typeof self !== 'undefined' ? self : null;

Because I want to polyfill requestAnimationFrame by add it as a global function. If we set _context to null when no self exists, the program would throw here:

three.js/src/renderers/common/Animation.js

Line 65 in d3ca4be

this._requestId = this._context.requestAnimationFrame( update );

[Mugen87]

Even with this polyfill is the renderer actually usable in a node or deno environment? Do you mind explaining your use case a bit?

[chirsz-ever]

Even with this polyfill is the renderer actually usable in a node or deno environment? Do you mind explaining your use case a bit?

Yes. I try to run three.js on Deno with WebGPU¹. the polyfill of requestAnimationFrame is

https://github.com/chirsz-ever/deno-webgpu-window-demos/blob/cc5666c17732a68a4dd4f0d8836cf4d771c27118/threejs/polyfill.ts#L151-L155

It is hard to resolve the problem with simplely assigning null to this._context, because Animation is not exported from three.webgpu.js (so we cannot modify its prototype ahead of time), and this._animation.start(); is almost right behind this._animation = new Animation(...) in Renderer.prototype.init(). This even cannot be resolved by set a self member on globalThis, because Deno would remove it when calling npm packages.

If this problem is fixed, most demos would work fine.

Footnotes

1. https://github.com/chirsz-ever/deno-webgpu-window-demos/blob/master/threejs ↩

[Mugen87]

How about moving the code into a new function in src/utils.js and call it getSelf(). In this way, we don't clutter Animation.js. Besides, we might need the same logic elsewhere.

I would not simply use globalThis to avoid issues in older environments or tools.

[chirsz-ever]

@Mugen87 getSelf has other meanings in the existing code, so I use getGlobalThis instead.

[chirsz-ever]

@Mugen87

I found another way to make my polyfill work (basically using code in src instead of build), so I switched to the simplest modification we both agreed on.

One more thing I want to request is, whether we should add Animation to the global exports? Just like add this line to src/Three.*.js:

 export { WebXRController } from './renderers/webxr/WebXRController.js';
+export { Animation } from './renderers/common/Animation.js';
 export { FogExp2 } from './scenes/FogExp2.js';

This would make it easier for me to polyfill and allow me to use the code in build.

If you don't agree to export Animation, you can just merge this PR, and I have no more opinions. Or if you allow to export Animation, I would like to add the code.

[Mugen87]

Animation.js is considered to be an internal class. If we export it, it becomes part of the public API which is not wanted. Sorry, but I think it's better to keep the PR as it is.