Nodes module uses class names to register nodes (addNodeClass) which crashes in production

[njarraud]

Description

The addNodeClass function exported from nodes/core/Nodes.js uses the name property of the nodes classes to register the nodes. The node module works well in development however when the code is packed for production, class names are often modified by the compiler and therefore the whole module crashes.

I believe that this function is mainly used by NodeLoader, but because of this test

if ( NodeClasses.has( nodeClass.name ) ) throw new Error( `Redefinition of node class ${ nodeClass.name }` );

it crashes as soon as any call to this module is made which makes it unusable for production.

Reproduction steps

1. Import the node module in a file and build for production e.g. webpack

Code

None

Live example

None

Screenshots

No response

Version

r155

Device

No response

Browser

No response

OS

No response

[njarraud]

Would it be acceptable to use a static property for each node class?

class BitangentNode extends Node {
    static type = "BitangentNode";
    …
}

and then for addNodeClass -

export function addNodeClass( nodeClass ) {

    if ( typeof nodeClass !== 'function' || ! nodeClass.type ) throw new Error( `Node class ${ nodeClass.type } is not a class` );
    if ( NodeClasses.has( nodeClass.type ) ) throw new Error( `Redefinition of node class ${ nodeClass.type }` );

    NodeClasses.set( nodeClass.type, nodeClass );

}

createNodeFromType would be working the same way.

Therefore, the code would not require predefined class names and the node loader would be unchanged.

[njarraud]

There is the same issue with addLightNode and addNodeMaterial.

[danrossi]

Terser removes the class name entirely. Here is my work around for now placing them in an exemption list.

https://github.com/danrossi/three-webgpu-renderer/blob/master/rollup.config.js#L75

[njarraud]

Terser settings are not always available. e.g. NextJS

[danrossi]

That does use Terser also so there should be a config for adding something like this

keep_classnames: /ArrayUniformNode/

[njarraud]

Nextjs actually now uses SWC by default but like for Terser, the options are handled internally by the framework and cannot be changed. I know that this is not threejs problem, but I have the feeling that this small change would make the life easier and save troubleshooting time for anybody willing to use the node system seriously in production at the moment.

[LeviPesin]

I think we can do it like so:

export function addNodeClass( nodeClass, name ) {

    if ( typeof nodeClass !== 'function' || ! name ) throw new Error( `Node class ${ name } is not a class` );
    if ( NodeClasses.has( name ) ) throw new Error( `Redefinition of node class ${ name }` );

    NodeClasses.set( name, nodeClass );

}

addNodeClass( BitangentNode, 'BitangentNode' );

@njarraud Would this solve your issue?

[njarraud]

Just looking at it, yes I believe I would work.

[sunag]

We must consider serialization in this fix as well.

three.js/examples/jsm/nodes/core/Node.js

Lines 29 to 33 in d930553

	get type() {
	return this.constructor.name;
	}

[verekia]

It's not much, but I am offering a $50 bounty for fixing WebGPU minification-related issues. Hopefully this is the only one!

That's the last missing bit for shipping WebGPU support to MiniMana.io :)

[verekia]

With r157, running in production is now causing a new kind of bug.

It works fine in development mode, but when I build the app in production mode (with Next.js), I am now getting:

Error: NodeMaterial: Material "MeshStandardMaterial" is not compatible.

This is coming from:
https://github.com/mrdoob/three.js/blob/181e04eea8b569dc09048f9dc644310ed6b745a6/examples/jsm/nodes/materials/NodeMaterial.js#L506C22-L506C84

Does that seem minification-related?

[verekia]

I'll send the bounty either way. @LeviPesin, is it fine to send the money to your GH sponsor account? If not please get in touch! On Twitter or Discord as verekia, or the email on verekia.com.

[LeviPesin]

Does that seem minification-related?

I think it could happen from tree-shaking node materials (and addNodeMaterial calls, maybe they could be somehow specifically marked as containing side-effects).

is it fine to send the money to your GH sponsor account?

Sure!