Skip to content

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Apr 2, 2025

Related issue: #30783

Description

Fix instance() optional parameter.

@sunag sunag requested a review from Copilot April 2, 2025 16:02
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes the instance() function by making the instanceColor parameter optional, while updating both the documentation and the parameter count settings accordingly.

  • Constructor now sets a default value (null) for the instanceColor parameter.
  • Documentation annotations have been updated to indicate its optional status.
  • The parameter length configuration has been revised using setParameterLength.
Comments suppressed due to low confidence (1)

src/nodes/accessors/InstanceNode.js:224

  • Ensure that setParameterLength(2, 3) accurately reflects the intended parameter requirements after making instanceColor optional, and that this change preserves backwards compatibility.
export const instance = /*@__PURE__*/ nodeProxy( InstanceNode ).setParameterLength( 2, 3 );

Copy link

github-actions bot commented Apr 2, 2025

📦 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.1
148.16
533.08
148.15
-17 B
-6 B
WebGPU Nodes 532.56
148.06
532.54
148.05
-17 B
-6 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.71
164.13
604.69
164.12
-19 B
-10 B
WebGPU Nodes 559.7
153.56
559.68
153.56
-19 B
-5 B

@sunag sunag marked this pull request as ready for review April 2, 2025 16:24
@sunag sunag merged commit b4ca470 into mrdoob:dev Apr 2, 2025
12 checks passed
@sunag sunag deleted the dev-fix-optional-parameter branch April 2, 2025 16:52
@sunag
Copy link
Collaborator Author

sunag commented Apr 2, 2025

/cc @brunosimon

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.

2 participants