P5.js: validateParameters using undefined property for validation

[rdelpeso]

Nature of issue?

• [x] Found a bug
• [ ] Existing feature enhancement
• [ ] New feature request

Most appropriate sub-area of p5.js?

• [ ] Color
• [x] Core
• [ ] Data
• [ ] Events
• [ ] Image
• [ ] IO
• [ ] Math
• [ ] Typography
• [ ] Utilities
• [ ] WebGL
• [ ] Other (specify if possible)

Which platform were you using when you encountered this?

• [ ] Mobile/Tablet (touch devices)
• [x] Desktop/Laptop
• [ ] Others (specify if possible)

Details about the bug:

• p5.js version: v0.6.0 January 19, 2018
• Web browser and version: Google Chrome | 63.0.3239.132 (Official Build) (64-bit) (cohort: Stable)
• Operating System: Windows 10 Pro
• Steps to reproduce this:

While using the non minified library from NPM there seems to be an issue inside validateParameters:

https://github.com/processing/p5.js/blob/bfc79d7b4c15801a2e2210933a0163c9d42b3b0e/src/core/error_helpers.js#L296

type.t
// Property t does not exist, instead now it seems to be called name or type. Example of Image:
// type.name: "p5.Image"
// type.type: "p5.image"

I don't understand the inner workings of validateParameters but I don't think this is working as expected.

Further more, typeof param returns object which will never match type.type which is p5.Image. So perhaps a better approach would be something like:

else if (param.name && type.name) {
  return param.name === type.name
}

And then fallback onto the typeof comparison versus type.type if the incoming param does not have a name property.

Thoughts? If someone confirms my findings I'll prepare a pull request.

[Spongman]

can you post simple example sketch that reproduces the problem you're seeing?

from your comments, i'm guessing you're passing a p5.Image to a function that expects an image, and it's giving you an error.

however, i'm not seeing the problem you describe with the following sketch:

var img;
function setup() {
  createCanvas(500, 500);
  img = createImage(100, 100);
}

function draw() {
  background(img);
}

the correct line in error_helpers.j that matches the img parameter is this:

} else if (type.prototype) {
  return param instanceof type.prototype;

maybe something else is going on here?

[rdelpeso]

You are correct, my initial finding was the result of an entirely different problem.

The way I have imported p5 prevented it from ever becoming available in window.p5, so the following line fails:

https://github.com/processing/p5.js/blob/bfc79d7b4c15801a2e2210933a0163c9d42b3b0e/src/core/error_helpers.js#L246

Since there is no window.p5 it bails out of there resulting on the code returning from here:
https://github.com/processing/p5.js/blob/bfc79d7b4c15801a2e2210933a0163c9d42b3b0e/src/core/error_helpers.js#L252

Instead of where it should have returned:
https://github.com/processing/p5.js/blob/bfc79d7b4c15801a2e2210933a0163c9d42b3b0e/src/core/error_helpers.js#L249

Which leads to the problem i described above. I'll try to put together a minimal example of how am setting up my project, but I suspect that it is quite a unique situation that not many people run into.

The situation seems to be that the lookupParamDoc function relies on window.p5, and my current implementation provides p5 in a closure.

For this one case, the simplest and dirtiest solution that comes to mind to support the situation is something like:

t = t && t[p];
// to
t = t && (t[p] || eval('p5'));

Yuck, i know. But it does work :(

For the time being i have adapted this solution to avoid the spam of messages:

//@ts-ignore
window.p5 = p5

The basic idea of my project is:

• Webpack + TypeScript + p5
• Import p5 as a module inside my "rendering module" using the instance mode of p5, and avoiding global scope polluting, not even window.p5
• Proxy all high level draw calls through my rendering module

This allows me to decouple the renderer from the library used to back it. Likely not a very common way of using p5. If you think this is far enough out of p5 scope we can close this ticket. If not I will provide a working repository with the absolute minimum required to reproduce the issue.

[rdelpeso]

I just cobbled together a minimal example that reproduces the behavior:

https://jsfiddle.net/ikatsuke/br96wmpz/

Edit: Look at the logs for the info log spam.

[Spongman]

ok, so this is the culprit in your sketch:

window.p5 = undefined

if you remove that line, everything works as expected.

[rdelpeso]

That fiddle is a very contrived example and needs that line in order to reproduce the problem. So it can not be removed.

If you import p5 as a module inside a typescript project, the window object never gets modified and there is no window.p5. So this fiddle reproduces the situation as close as possible in a working fashion.

Here is an excerpt of my typescript project:
```Renderer.ts
// Magic for importing p5 NPM package properly in TypeScript and Webpack
import 'p5'; var p5 = require("p5");

const s: p5 = new p5(() => { });

const data = {
data: s.createImage(100, 100)
};

s.setup = () => {
s.createCanvas(500, 500);
}

s.draw = () => {
s.image(data.image, 0, 0, 100, 100);
}

console.log(window.p5);
// undefined
```

[Spongman]

can you try with this: https://github.com/processing/p5.js/pull/2582 ?

[Spongman]

i think providing a sample of bundling p5 with webpack would be useful nonetheless.

[rdelpeso]

I can confirm that with that patch the issue no longer occurs. And is a nice way to solve for p5 being in scope, even if not within window!

I will add a repo in a few minutes that has a few examples of using p5 with webpack and/or typescript. And post a link here for reference.

Thanks for your help!

[rdelpeso]

Here we go: https://github.com/rdelpeso/p5-examples

I plan to keep adding samples of skeleton setups to use p5 as i think of them and time permits. For now you could use the simple-webpack sample to validate the situation on this ticket. All you'd need to do is add an image on setup and try to render it on draw, and the spam of info logs occurs given p5 v0.6.0 from NPM.