Skip to content

[dev-2.0] Fix optional and rest parameters in TypeScript class method declarations #7863

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: dev-2.0
Choose a base branch
from
Open

[dev-2.0] Fix optional and rest parameters in TypeScript class method declarations #7863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7d5985f
[dev-2.0] Fix optional and rest parameters in TypeScript class method…
SoundOfScooting Jun 2, 2025
1b7e808
[dev-2.0] Fix `@chainable` in class method declarations
SoundOfScooting Jun 15, 2025
5e33081
Fix doc comments
SoundOfScooting Jul 11, 2025
ad44d73
Document VIDEO and AUDIO constants
SoundOfScooting Jul 11, 2025
85f59cc
Fix syntax, accessors, descriptions
SoundOfScooting Jul 11, 2025
a85660a
Fix some issues with constants
SoundOfScooting Jul 11, 2025
49b4eae
patch.mjs draft
SoundOfScooting Aug 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 22 additions & 15 deletions docs/parameterData.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@
"applyMatrix": {
"overloads": [
[
"Array"
"Number[]"
],
[
"Number",
Expand Down Expand Up @@ -1300,9 +1300,6 @@
"Number",
"Number",
"Number"
],
[
"p5.Vector"
]
]
},
Expand Down Expand Up @@ -2602,7 +2599,7 @@
"imageLight": {
"overloads": [
[
"p5.image"
"p5.Image"
]
]
},
Expand Down Expand Up @@ -3052,7 +3049,6 @@
},
"createAudio": {
"overloads": [
[],
[
"String|String[]?",
"Function?"
Expand Down Expand Up @@ -4128,6 +4124,9 @@
},
"mult": {
"overloads": [
[
"Number"
],
[
"Number",
"Number",
Expand Down Expand Up @@ -4236,6 +4235,18 @@
]
]
},
"dist": {
"overloads": [
[
"p5.Vector"
],
[],
[
"p5.Vector",
"p5.Vector"
]
]
},
"normalize": {
"overloads": [
[],
Expand Down Expand Up @@ -4385,6 +4396,11 @@
]
]
},
"clampToZero": {
"overloads": [
[]
]
},
"fromAngle": {
"overloads": [
[
Expand All @@ -4411,15 +4427,6 @@
"overloads": [
[]
]
},
"dist": {
"overloads": [
[],
[
"p5.Vector",
"p5.Vector"
]
]
}
},
"p5.Font": {
Expand Down
10 changes: 8 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,14 @@
"license": "LGPL-2.1",
"browser": "./lib/p5.min.js",
"exports": {
".": "./dist/app.js",
"./core": "./dist/core/main.js",
".": {
Copy link
Contributor

Choose a reason for hiding this comment

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

@limzykenneth I think you updated type exporting in package.json last, does this look OK to you?

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be neccessary the update all the exports fields and additionally add the field "type": "module" to package.json. This affects the esm build not the iife. By using the exports condition default the files will be interpreted as .cjs files which might also result in conflicts with the type definitions. I'll doublecheck later
see

Copy link
Author

Choose a reason for hiding this comment

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

The link you posted says default "Can be a CommonJS or ES module file".

Also, forgot to mention this here in the PR, but the other index.js exports fields don't have corresponding index.d.ts declaration files to point to. I think it's because those files don't have any doc comments themselves.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if this should be part of a separate issue.

Please correct me if I'm wrong because I'm still in the process of wrapping my head around generate-types.mjs and reading relevant issues.

I'm using publint and arethetypeswrong as a reference. Generally these tools are made to check node.js packages therefore only the bundler module resolution should be of interest. Nonetheless there are some resolution errors within the declaration files. E.g.:

  • p5.d.ts references itself /// <reference path="./p5.d.ts" />
  • p5.Color.d.ts imports itself import { Color } from '../color/p5.Color'; (There are similar issues in other .d.ts files) I assume it's due to circular dependencies (as stated in the rollup build logs)

Regarding the conditional exports:

  • "./friendlyErrors": "./dist/core/friendlyErrors/index.js", can't be resolved. The correct file path would be "./dist/core/friendly_errors/index.js"

Yes the link I posted refers to node's module resolution algorithm whereas the majority of the p5.js endusers and consumers will or should use a bundler to resolve modules. So the main take away of this link is:

Writing ES module syntax in "ambiguous" files incurs a performance cost, and therefore it is encouraged that authors be explicit wherever possible. In particular, package authors should always include the "type" field in their package.json files, even in packages where all sources are CommonJS. Being explicit about the type of the package will future-proof the package in case the default type of Node.js ever changes, and it will also make things easier for build tools and loaders to determine how the files in the package should be interpreted.

https://nodejs.org/api/packages.html#introduction_1

Copy link
Author

@SoundOfScooting SoundOfScooting Jun 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I agree, it probably makes more sense as a separate issue then. I thought the problem would only require a small change so I was hoping to get it merged with the overload fix at the same time. If requested I will remove the change from this PR for a future one.

Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing for the other exports we don't have a setup for their types just yet that would make sense? We don't need it now as the full ESM modular use is not at a stage where I want to push it publicly yet, but we can start thinking about how it would work.

I'm fully in favor of including "type": "module" in package.json and shift any remaining code we have that still uses cjs to ESM, I think dependencies wise it should all support ESM without issue now.

"types": "./types/p5.d.ts",
"default": "./dist/app.js"
},
"./core": {
"types": "./types/core/main.d.ts",
"default": "./dist/core/main.js"
},
"./shape": "./dist/shape/index.js",
"./accessibility": "./dist/accessibility/index.js",
"./friendlyErrors": "./dist/core/friendlyErrors/index.js",
Expand Down
2 changes: 1 addition & 1 deletion src/core/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,7 @@ export const POINTS = 0x0000;
*/
export const LINES = 0x0001;
/**
* @property {0x0003} LINE_STRIP
* @typedef {0x0003} LINE_STRIP
* @property {LINE_STRIP} LINE_STRIP
* @final
*/
Expand Down
1 change: 1 addition & 0 deletions src/core/friendly_errors/param_validator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ function validateParams(p5, fn, lifecycles) {
* parameters, and `?` is a shorthand for `Optional`.
*
* @method generateZodSchemasForFunc
* @private
* @param {String} func - Name of the function. Expect global functions like `sin` and class methods like `p5.Vector.add`
* @returns {z.ZodSchema} Zod schema
*/
Expand Down
2 changes: 1 addition & 1 deletion src/core/transform.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function transform(p5, fn){
* cause shapes to transform continuously.
*
* @method applyMatrix
* @param {Array} arr an array containing the elements of the transformation matrix. Its length should be either 6 (2D) or 16 (3D).
* @param {Array<Number>} arr an array containing the elements of the transformation matrix. Its length should be either 6 (2D) or 16 (3D).
* @chainable
*
* @example
Expand Down
29 changes: 21 additions & 8 deletions src/dom/p5.MediaElement.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,19 @@

import { Element } from './p5.Element';

/**
* @typedef {'video'} VIDEO
* @property {VIDEO} VIDEO
* @final
*/
export const VIDEO = 'video';
/**
* @typedef {'audio'} AUDIO
* @property {AUDIO} AUDIO
* @final
*/
export const AUDIO = 'audio';

class MediaElement extends Element {
constructor(elt, pInst) {
super(elt, pInst);
Expand Down Expand Up @@ -1318,7 +1331,7 @@ function media(p5, fn){
return c;
}

/** VIDEO STUFF **/
/*** VIDEO STUFF ***/

// Helps perform similar tasks for media element methods.
function createMedia(pInst, type, src, callback) {
Expand Down Expand Up @@ -1457,10 +1470,10 @@ function media(p5, fn){
*/
fn.createVideo = function (src, callback) {
// p5._validateParameters('createVideo', arguments);
return createMedia(this, 'video', src, callback);
return createMedia(this, VIDEO, src, callback);
};

/** AUDIO STUFF **/
/*** AUDIO STUFF ***/

/**
* Creates a hidden `&lt;audio&gt;` element for simple audio playback.
Expand Down Expand Up @@ -1504,14 +1517,14 @@ function media(p5, fn){
*/
fn.createAudio = function (src, callback) {
// p5._validateParameters('createAudio', arguments);
return createMedia(this, 'audio', src, callback);
return createMedia(this, AUDIO, src, callback);
};

/** CAMERA STUFF **/
/*** CAMERA STUFF ***/

fn.VIDEO = 'video';
fn.VIDEO = VIDEO;

fn.AUDIO = 'audio';
fn.AUDIO = AUDIO;

// from: https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/getUserMedia
// Older browsers might not implement mediaDevices at all, so we set an empty object first
Expand Down Expand Up @@ -1693,7 +1706,7 @@ function media(p5, fn){

const videoConstraints = { video: useVideo, audio: useAudio };
constraints = Object.assign({}, videoConstraints, constraints);
const domElement = document.createElement('video');
const domElement = document.createElement(VIDEO);
// required to work in iOS 11 & up:
domElement.setAttribute('playsinline', '');
navigator.mediaDevices.getUserMedia(constraints).then(function (stream) {
Expand Down
3 changes: 0 additions & 3 deletions src/math/p5.Vector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,6 @@ class Vector {
* <a href="#/p5.Vector">p5.Vector</a> object and doesn't change the
* originals.
*
* @method mult
* @param {Number} n The number to multiply with the vector
* @chainable
* @example
Expand Down Expand Up @@ -1591,7 +1590,6 @@ class Vector {
* Use <a href="#/p5/dist">dist()</a> to calculate the distance between points
* using coordinates as in `dist(x1, y1, x2, y2)`.
*
* @method dist
* @submodule p5.Vector
* @param {p5.Vector} v x, y, and z coordinates of a <a href="#/p5.Vector">p5.Vector</a>.
* @return {Number} distance.
Expand Down Expand Up @@ -3060,7 +3058,6 @@ class Vector {
*
* https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/EPSILON
*
* @method clampToZero
* @return {p5.Vector} with components very close to zero replaced with zero.
* @chainable
*/
Expand Down
2 changes: 1 addition & 1 deletion src/webgl/light.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -895,7 +895,7 @@ function light(p5, fn){
* use as the light source.
*
* @method imageLight
* @param {p5.image} img image to use as the light source.
* @param {p5.Image} img image to use as the light source.
*
* @example
* <div class="notest">
Expand Down
7 changes: 4 additions & 3 deletions src/webgl/p5.Quat.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class Quat {
* Returns a Quaternion for the
* axis angle representation of the rotation
*
* @method fromAxisAngle
* @private
* @param {Number} [angle] Angle with which the points needs to be rotated
* @param {Number} [x] x component of the axis vector
* @param {Number} [y] y component of the axis vector
Expand All @@ -34,7 +34,7 @@ class Quat {

/**
* Multiplies a quaternion with other quaternion.
* @method mult
* @private
* @param {p5.Quat} [quat] quaternion to multiply with the quaternion calling the method.
* @chainable
*/
Expand All @@ -55,6 +55,7 @@ class Quat {
* the multiplication can be simplified to the below formula.
* This was taken from the below stackexchange link
* https://gamedev.stackexchange.com/questions/28395/rotating-vector3-by-a-quaternion/50545#50545
* @private
* @param {p5.Vector} [p] vector to rotate on the axis quaternion
*/
rotateVector(p) {
Expand All @@ -68,7 +69,7 @@ class Quat {
* Rotates the Quaternion by the quaternion passed
* which contains the axis of roation and angle of rotation
*
* @method rotateBy
* @private
* @param {p5.Quat} [axesQuat] axis quaternion which contains
* the axis of rotation and angle of rotation
* @chainable
Expand Down
13 changes: 11 additions & 2 deletions src/webgl/text.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ function text(p5, fn) {

/**
* @function setPixel
* @private
* @param {Object} imageInfo
* @param {Number} r
* @param {Number} g
Expand Down Expand Up @@ -230,6 +231,7 @@ function text(p5, fn) {

/**
* @function push
* @private
* @param {Number[]} xs the x positions of points in the curve
* @param {Number[]} ys the y positions of points in the curve
* @param {Object} v the curve information
Expand All @@ -242,6 +244,7 @@ function text(p5, fn) {

/**
* @function minMax
* @private
* @param {Number[]} rg the list of values to compare
* @param {Number} min the initial minimum value
* @param {Number} max the initial maximum value
Expand Down Expand Up @@ -291,6 +294,7 @@ function text(p5, fn) {

/**
* @function clamp
* @private
* @param {Number} v the value to clamp
* @param {Number} min the minimum value
* @param {Number} max the maxmimum value
Expand All @@ -305,6 +309,7 @@ function text(p5, fn) {

/**
* @function byte
* @private
* @param {Number} v the value to scale
*
* converts a floating-point number in the range 0-1 to a byte 0-255
Expand Down Expand Up @@ -440,6 +445,7 @@ function text(p5, fn) {

/**
* @function cubicToQuadratics
* @private
* @param {Number} x0
* @param {Number} y0
* @param {Number} cx0
Expand Down Expand Up @@ -508,6 +514,7 @@ function text(p5, fn) {

/**
* @function pushLine
* @private
* @param {Number} x0
* @param {Number} y0
* @param {Number} x1
Expand All @@ -523,6 +530,7 @@ function text(p5, fn) {

/**
* @function samePoint
* @private
* @param {Number} x0
* @param {Number} y0
* @param {Number} x1
Expand Down Expand Up @@ -608,9 +616,10 @@ function text(p5, fn) {

/**
* @function layout
* @private
* @param {Number[][]} dim
* @param {ImageInfo[]} dimImageInfos
* @param {ImageInfo[]} cellImageInfos
* @param {ImageInfos} dimImageInfos
* @param {ImageInfos} cellImageInfos
* @return {Object}
*
* lays out the curves in a dimension (row or col) into two
Expand Down
7 changes: 5 additions & 2 deletions utils/generate-types.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import fs from 'fs';
import path from 'path';
import { fileURLToPath } from 'url';
import {
generateTypeDefinitions
generateTypeDefinitions,
normalizeIdentifier
} from "./helper.mjs";

// Fix for __dirname equivalent in ES modules
Expand Down Expand Up @@ -53,7 +54,7 @@ export function generateAllDeclarationFiles() {
`${parsedPath.name}.d.ts`
);

const exportName = parsedPath.name.replace('.', '_');
const exportName = normalizeIdentifier(parsedPath.name.replace('.', '_'));
const contentWithExport = content + `export default function ${exportName}(p5: any, fn: any): void;\n`;

fs.mkdirSync(path.dirname(dtsPath), { recursive: true });
Expand All @@ -68,6 +69,8 @@ export function generateAllDeclarationFiles() {
// Add references to all other .d.ts files
const dtsFiles = findDtsFiles(path.join(__dirname, '..'));
for (const file of dtsFiles) {
if (file === 'p5.d.ts')
continue;
p5Types += `/// <reference path="./${file}" />\n`;
}
p5Types += '\n';
Expand Down
Loading