Skip to content

Node: Clarify Usage of Setup in Function Declaration #31535

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

Merged
merged 2 commits into from
Jul 31, 2025

Conversation

cmhhelgeson
Copy link
Contributor

Description

The comment for the base Node setup class currently reads as follows:

/**
 * Represents the setup stage which is the first step of the build process, see {@link Node#build} me
 * This method is often overwritten in derived modules to prepare the node which is used as the output
 * The output node must be returned in the `return` statement.
 *
 * @param {NodeBuilder} builder - The current node builder.
 * @return {?Node} The output node.
 */
setup( builder ) {

For some, this can be misconstrued as all setup nodes requiring a returned outputNode. However, in many nodes, this is not the case. Examples include ConditionalNode, LoopNode, LightProbeNode, IrradianceNode, HemisphereLightNode, AONode, AnalyticLightNode, VaryingNode, ContextNode, and more. These nodes often perform actions that are not captured by the description in the setup function declaration, including modifying the context or creating new nodes. Additionally, many of these node are built under the assumption that setup() will not return and will fallback to properties.outputNode or null with Node.js

// Node.js

if ( buildStage === 'setup' ) {
	this.updateReference( builder );
	const properties = builder.getNodeProperties( this );
	if ( properties.initialized !== true ) {
		//const stackNodesBeforeSetup = builder.stack.nodes.length;
		properties.initialized = true;
		properties.outputNode = this.setup( builder ) || properties.outputNode || null;

Accordingly, I think the declaration for the setup function should more explicitly indicate that a returned outputNode is not necessary for every setup declaration.

Copy link

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 338.82
79.08
338.82
79.08
+0 B
+0 B
WebGPU 566.16
156.5
566.16
156.5
+0 B
+0 B
WebGPU Nodes 564.77
156.26
564.77
156.26
+0 B
+0 B

🌳 Bundle size after tree-shaking

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

Before After Diff
WebGL 470.25
113.78
470.25
113.78
+0 B
+0 B
WebGPU 637.38
172.47
637.38
172.47
+0 B
+0 B
WebGPU Nodes 592.02
161.71
592.02
161.71
+0 B
+0 B

@sunag sunag changed the title Node - Clarify Usage of Setup in Function Declaration Node: Clarify Usage of Setup in Function Declaration Jul 31, 2025
@sunag sunag merged commit 5588622 into mrdoob:dev Jul 31, 2025
9 checks passed
@mrdoob mrdoob added this to the r179 milestone Jul 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants