[p5.js 2.0 Beta Bug Report]: createVector(x, y, z) shows FES message

[davepagurek]

Most appropriate sub-area of p5.js?

• Accessibility
• Color
• Core/Environment/Rendering
• Data
• DOM
• Events
• Image
• IO
• Math
• Typography
• Utilities
• WebGL
• Build process
• Unit testing
• Internationalization
• Friendly errors
• Other (specify if possible)

p5.js version

Latest dev-2.0

Web browser and version

Firefox

Operating system

MacOS

Steps to reproduce this

Steps:

Calling createVector(x, y, z) currently shows this message:

p5.js says: Expected at most 1 argument, but received more in createVector(). For more information, see https://p5js.org/reference/p5/createVector. 

Looks like maybe the doc comments are out of date on createVector?

Snippet:

function setup() {
  createCanvas(200, 200)
  createVector(1, 2, 3)
}

[davepagurek]

Looks like it's happening because of this line in the createVector docs:

p5.js/src/math/math.js

Line 37 in 55ed78e

* @param {...Number} components Components of the vector.

We currently don't parse this spread notation correctly, treating it like a single positional argument:

So for now we could document x, y, and z separately like in 1.x:

p5.js/src/math/math.js

Lines 34 to 36 in 57638a0

	* @param {Number} [x] x component of the vector.
	* @param {Number} [y] y component of the vector.
	* @param {Number} [z] z component of the vector.

...longer term, we probably need to get ... working for spread params.

[GregStanton]

Thanks @davepagurek!

Do we need to update the overloads for createVector() in parameterData.json? The contributor docs say that FES does parameter validation based on docs/reference/data.json, but that doesn't seem to exist anymore, and I'm not familiar with how this works.

To address the documentation itself, maybe we could leave it as it is, and temporarily describe components as "the components of the vector, as individual number arguments"? It's awkward, but it seems more accurate than only documenting $x$, $y$, and $z$ when $n$ dimensions are actually supported. (Although max() is another variadic function that's documented with this kind of workaround, I think it's similarly misleading there.)

Maybe after the immediate problem is resolved with a workaround, a new issue could be created to resolve the underlying problem with the JSDoc syntax for rest parameters not being parsed correctly?

[davepagurek]

@GregStanton When we make a build of p5, parameterData.json gets rewritten automatically from the docs when npm run docs is called (part of the build process, but can also be run manually if you want to preview the effects of a change.) So we unfortunately can't leave the docs as is since they're linked to parameterData.json, which is what needs to change for FES to work.

Maybe after the immediate problem is resolved with a workaround, a new issue could be created to resolve the underlying problem with the JSDoc syntax for rest parameters not being parsed correctly?

I think this makes sense! We'll likely also have to make an accompanying website issue too so that the syntax section of the reference looks like createVector(...components) instead of createVector(components) to avoid it looking like a single parameter should be passed in.

[GregStanton]

Thanks for the explanation @davepagurek! I'm enjoying learning about how the reference is built.

When we make a build of p5, parameterData.json gets rewritten automatically from the docs when npm run docs is called

That makes more sense. It'd be a maintenance problem if parameterData.json had to be set manually. I was a bit confused since I thought I saw manual commits that had been made to it, and also because some of the entries didn't seem to match the code... But maybe the commits were due to npm run docs, and I think the mismatch I perceived was because I was looking at an old version of the file 😅

We'll likely also have to make an accompanying website issue too so that the syntax section of the reference looks like createVector(...components) instead of createVector(components)

Oh, I see. That'd definitely be better. It might be a bit confusing, though? For beginners looking to use a simple function like max(), it'd be good if we don't assume familiarity with rest parameters or spread syntax. Also, users often wouldn't call the function with spread syntax. So this approach would require having an accurate parameter description, in order to explain that any number of individual parameters can be passed in. That's workable, but as a reference-wide approach, it'd be hard to enforce good parameter descriptions all the time.

Could we get it to look like createVector(x1, x2, ..., xn)? That's similar to how MDN documents variadic functions like Math.max(). They replace ..., with /*...,*/ so that it's valid syntax, but I think that might be a bit more complicated for beginners. We already use brackets to indicate optional parameters, which isn't valid syntax; that might not be a very good argument by itself, but I don't think there's much room for misinterpretation with an uncommented ellipsis. (It couldn't be a rest parameter or a spread operator in this context.)