small sample of TS-compatible type definitions for PosterizeNode.js #4
Conversation
…e type definitions
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
There was a problem hiding this comment.
Pull request overview
This PR proposes adding TypeScript-compatible type definitions to PosterizeNode.js as a sample implementation. The changes aim to improve TypeScript compatibility by introducing JSDoc @import syntax for type references and using @type annotations with function signatures instead of @function tags.
Key changes:
- Adds a JSDoc @import statement to reference the Node type from the core module
- Replaces the @function tag with a @type annotation that includes the full function signature for the posterize TSL function
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,5 +1,6 @@ | |||
| import TempNode from '../core/TempNode.js'; | |||
| import { nodeProxy } from '../tsl/TSLBase.js'; | |||
| /** @import Node from '../core/Node.js' */ | |||
There was a problem hiding this comment.
There is an extra space before the closing comment delimiter. The correct JSDoc @import syntax should have no extra spaces.
| /** @import Node from '../core/Node.js' */ | |
| /** @import Node from '../core/Node.js' */ |
| * @param {Node} sourceNode - The input color. | ||
| * @param {Node} stepsNode - Controls the intensity of the posterization effect. A lower number results in a more blocky appearance. | ||
| * @returns {PosterizeNode} |
There was a problem hiding this comment.
When using @type to define a function signature, the @param and @returns tags become redundant and may confuse TypeScript-compatible tooling. When @type fully describes the function signature including parameters and return type, the separate @param and @returns tags should be removed to avoid duplication and potential inconsistency.
| * @param {Node} sourceNode - The input color. | |
| * @param {Node} stepsNode - Controls the intensity of the posterization effect. A lower number results in a more blocky appearance. | |
| * @returns {PosterizeNode} |
| @@ -1,5 +1,6 @@ | |||
| import TempNode from '../core/TempNode.js'; | |||
| import { nodeProxy } from '../tsl/TSLBase.js'; | |||
| /** @import Node from '../core/Node.js' */ | |||
There was a problem hiding this comment.
This @import syntax is not used anywhere else in the codebase and introduces a new pattern that is inconsistent with the existing documentation style. All other node files use @type annotations for Node references without @import statements. Additionally, Node types are already available through existing imports, making this @import unnecessary for JSDoc type checking.
| /** @import Node from '../core/Node.js' */ |
1 participant
small sample of making everything in PosterizeNode.js have TS-compatible type definitions
Related issue: #XXXX
Description
Shows how to format definitions that are compatible in downstream TS projects.
I have checked the doc generator, but this change may possibly break compatibility with that if the doc tool does not support TS-compatible JSDoc. If that's the case, we would need to either update that tool (possibly lots of work) or choose a different tool.