Remove IIFEs, to loosen cyclical dependencies

[Rich-Harris]

Following @Mugen87's suggestion in #14654 (comment):

A number of classes have methods that are returned from immediately-invoked function expressions (IIFEs), meaning that helper instances are created eagerly. This creates cyclical dependencies that make it tricky to convert the entire codebase to classes, and impede tree-shaking.

Because the codebase is composed of modules, we can simply hoist the helper variable declarations to the top of the modules that use them, and instantiate them lazily when the methods are invoked.

[takahirox]

You don't need to include build/* in PR.

[Mugen87]

What about the methods listed below? They have the following notation (note the missing pair of brackets):

distanceToPoint: function () {

    var v1 = new Vector2();

    return function distanceToPoint( point ) {

        var clampedPoint = v1.copy( point ).clamp( this.min, this.max );
        return clampedPoint.sub( point ).length();

    };

}(),

Do we also have to convert them?

Vector3.applyEuler(), Vector3.applyAxisAngle(), Vector3.clampScalar(), Vector3.unproject(), Vector3.projectOnPlane(), Vector3.reflect()

Object3D.rotateOnAxis(), Object3D.rotateOnWorldAxis(), Object3D.rotateX(), Object3D.rotateY(), Object3D.rotateZ(), Object3D.translateOnAxis(), Object3D.translateX(), Object3D.translateY(), Object3D.translateZ(), Object3D.worldToLocal(), Object3D.lookAt(), Object3D. getWorldQuaternion(), Object3D. getWorldScale()

PropertyBinding.sanitizeNodeName()

Camera.getWorldDirection()

BufferGeometry.rotateX(), BufferGeometry.rotateY(), BufferGeometry.rotateZ(), BufferGeometry.translate(), BufferGeometry.scale(), BufferGeometry.lookAt(), BufferGeometry.center(), BufferGeometry.normalizeNormals(), BufferGeometry.computeBoundingSphere()

Geometry.rotateX(), Geometry.rotateY(), Geometry.rotateZ(), Geometry.translate(), Geometry.scale(), Geometry.lookAt(), Geometry.center()

Box2.setFromCenterAndSize(), Box2.distanceToPoint()

Box3.setFromCenterAndSize(), Box3.expandByObject(), Box3.distanceToPoint()

Color.setHSL(), Color.copySRGBToLinear(), Color.copyLinearToSRGB(), Color.offsetHSL()

Euler.setFromQuaternion(), Euler.reorder()

Frustum.intersectsObject(), Frustum.intersectsSprite(), Frustum.intersectsBox()

Line3.closestPointToPointParameter()

Matrix3.applyToBufferAttribute()

Matrix4.extractRotation(), Matrix4.makeRotationFromQuaternion(), Matrix4.lookAt(), Matrix4.applyToBufferAttribute(), Matrix4.decompose()

Plane.intersectLine(), Plane.setFromCoplanarPoints(), Plane.applyMatrix4()

Quaternion.setFromUnitVectors()

Ray.recast(), Ray.distanceSqToPoint(), Ray.distanceSqToSegment(), Ray.intersectSphere(), Ray.intersectTriangle()

Sphere.setFromPoints()

Triangle.getNormal(), Triangle.getBarycoord(), Triangle.containsPoint(), Triangle.closestPointToPoint()

Vector2.clampScalar()

Vector3.applyEuler(), Vector3.applyAxisAngle(), Vector3.project(), Vector3.unproject(), Vector3.clampScalar(), Vector3.projectOnPlane(), Vector3.reflect()

Vector4.clampScalar()

LOD.update()

[Rich-Harris]

@takahirox understood — removed.

@Mugen87 argh, I was searching for )(), completely missed }(). Yes, they also need to be converted. There's enough of them that I might try and automate it...

In a situation like this one...

	rotateX: function () {

		// rotate geometry around world x-axis

		var m1 = new Matrix4();

		return function rotateX( angle ) {

			m1.makeRotationX( angle );

			this.applyMatrix( m1 );

			return this;

		};

	}(),

	rotateY: function () {

		// rotate geometry around world y-axis

		var m1 = new Matrix4();

		return function rotateY( angle ) {

			m1.makeRotationY( angle );

			this.applyMatrix( m1 );

			return this;

		};

	}(),

...does each method need its own instance? Or can they all use a shared m1?

[Mugen87]

...does each method need its own instance? Or can they all use a shared m1?

A method might call another method of the same class that both use the same type of temporary helper object. If we would now have a single shared instance, intermediate results get overwritten. An example for this is Triangle.containsPoint() which calls Triangle.getBarycoord(). Both have a v1 vector object in their closure scope.

three.js/src/math/Triangle.js

Lines 50 to 109 in a8e5062

	getBarycoord: function () {
	var v0 = new Vector3();
	var v1 = new Vector3();
	var v2 = new Vector3();
	return function getBarycoord( point, a, b, c, target ) {
	v0.subVectors( c, a );
	v1.subVectors( b, a );
	v2.subVectors( point, a );
	var dot00 = v0.dot( v0 );
	var dot01 = v0.dot( v1 );
	var dot02 = v0.dot( v2 );
	var dot11 = v1.dot( v1 );
	var dot12 = v1.dot( v2 );
	var denom = ( dot00 * dot11 - dot01 * dot01 );
	if ( target === undefined ) {
	console.warn( 'THREE.Triangle: .getBarycoord() target is now required' );
	target = new Vector3();
	}
	// collinear or singular triangle
	if ( denom === 0 ) {
	// arbitrary location outside of triangle?
	// not sure if this is the best idea, maybe should be returning undefined
	return target.set( - 2, - 1, - 1 );
	}
	var invDenom = 1 / denom;
	var u = ( dot11 * dot02 - dot01 * dot12 ) * invDenom;
	var v = ( dot00 * dot12 - dot01 * dot02 ) * invDenom;
	// barycentric coordinates must always sum to 1
	return target.set( 1 - u - v, v, u );
	};
	}(),
	containsPoint: function () {
	var v1 = new Vector3();
	return function containsPoint( point, a, b, c ) {
	Triangle.getBarycoord( point, a, b, c, v1 );
	return ( v1.x >= 0 ) && ( v1.y >= 0 ) && ( ( v1.x + v1.y ) <= 1 );
	};
	}()

BTW: These two methods are something special since they are directly assigned to the function constructor (not the prototype). In context of classes, these methods should ideally use the static keyword so they become static class methods.

[Mugen87]

@Rich-Harris I think we could also change the mentioned methods by hand. If you like, I can support you in this task.

[Rich-Harris]

I've made good progress on the automated conversion but haven't had a chance to finish it off this week — I was thinking that a manual conversion in those cases probably makes more sense, yes (otherwise we'd have a bunch of auto-generated v1_1, v1_2 etc). I'll update the PR soon

[bhouston]

I think you should use some type of prefix notation to denote these module scope variables from local function variables, otherwise people will make mistakes. Could be as simple as _position or m_position (flashbacks to MFC) or [functionName]_position would also be nice to reduce the scope of each variable to a specific function.

[marcofugaro]

What happened to this awesome PR? This is the first step to make the tree.js bundle tree-shakable

[Mugen87]

Closing. All IIFEs are now removed from the code base.