processing / p5.js

p5.js is a client-side JS platform that empowers artists, designers, students, and anyone to learn to code and express themselves creatively on the web. It is based on the core principles of Processing. http://twitter.com/p5xjs —
http://p5js.org/
GNU Lesser General Public License v2.1
21.66k stars 3.32k forks source link

Typescript cannot find changed() and input() methods on p5.Element from p5.dom #6364

Open hasnainroopawalla opened 1 year ago

hasnainroopawalla commented 1 year ago

Most appropriate sub-area of p5.js?

p5.js version

1.7.0

Web browser and version

Edge 115.0.1901.203

Operating System

MacOS 13.4.1

Steps to reproduce this

I'm working a node js project with p5 and typescript. In my sketch.ts, I'm trying to create an onChanged listener for a slider. FYI: I'm using p5 in instance mode.

sketch.ts

import * as p5 from "p5";

let world: World;

const maxSpeedSliderOnChanged = () => {
  alert("maxSpeedSlider changed");
};

const sketch = (p: p5) => {
  p.setup = () => {
    p.createCanvas(p.windowWidth, p.windowHeight);

    world = new World();

    const maxSpeedSlider = p.select("#maxSpeed");
    maxSpeedSlider.changed(maxSpeedSliderOnChanged); // Property 'changed' does not exist on type 'Element'.
  };

  p.draw = () => {
    world.render();
  };
};

export const p5i = new p5(sketch, document.body);

Functionally, this works as expected, however, typescript complains Property 'changed' does not exist on type 'Element'.

package.json

{
  "name": "test-project",
  "version": "0.0.4",
  "private": true,
  "scripts": {
    "start": "webpack serve --open",
    "build": "webpack",
    "predeploy": "npm run build",
    "deploy": "gh-pages -d build",
    "test": "jest",
    "test:watch": "jest --watch"
  },
  "keywords": [],
  "author": "",
  "license": "MIT",
  "devDependencies": {
    "@types/jest": "^29.5.3",
    "@types/p5": "^1.6.2",
    "css-loader": "^6.8.1",
    "gh-pages": "^5.0.0",
    "html-webpack-plugin": "^5.5.3",
    "jest": "^29.6.2",
    "jest-environment-jsdom": "^29.6.2",
    "p5": "^1.7.0",
    "style-loader": "^3.3.3",
    "ts-jest": "^29.1.1",
    "ts-loader": "^9.4.4",
    "typescript": "^5.1.6",
    "webpack": "^5.88.2",
    "webpack-cli": "^5.1.4",
    "webpack-dev-server": "^4.15.1"
  }
}

global.d.ts

import module = require("p5");
export = module;
export as namespace p5;

From dom.d.ts it is visible that changed() and input() etc. are all part of p5InstanceExtensions instead of Element.

Is this incorrectly configured or am I missing a shim for p5.dom?

Thanks!

welcome[bot] commented 1 year ago

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, please make sure to fill out the inputs in the issue forms. Thank you!

davepagurek commented 1 year ago

Since these classes are still using the prototype syntax, maybe we need @for p5.Element in the jsdoc comments for these methods? Or possibly we could try to refactor them to use classes like in other parts of p5

hasnainroopawalla commented 1 year ago

Hey @davepagurek, thanks for the response. Is this something that will be addressed in a future update?

limzykenneth commented 1 year ago

With the latest changes of moving things over to use class syntax only does this still apply? If we still need to add the @for syntax we can certainly try but we are not technically using JSDoc so not sure if it will work or not.

lindapaiste commented 11 months ago

This is probably an issue for the DefinitelyTyped repo, as they maintain the @types/p5 package.

davepagurek commented 11 months ago

Oops looks like I missed this, @limzykenneth I think that particular method isn't using the class syntax currently, so nothing in the metadata links it to the Element class unless we specify in the docs comments: https://github.com/processing/p5.js/blob/37d3324b457b177319ed65468201f2806a66eff5/src/dom/dom.js#L366-L369

@lindapaiste this is true, although we can make it easier for them by making sure our docs metadata is more consistent. Right now, I think they'd have to manually fix these methods, knowing that the docs metadata is incorrectly not assigning these methods to the right class.

This is beyond the scope of this particular issue, but @limzykenneth in the doc build system updates you're looking into, would it be compatible with typescript's jsdoc parser? It might be simpler overall to have first-party support for a .d.ts file, if it fits with the syntax we're already using.

limzykenneth commented 11 months ago

I forgot about this issue actually so didn't look into it. Both JSDoc and Documentation.js can produce something out of the current inline docs that we have (something because I didn't check how correct they are), have not yet tried with tsc so cannot say if it works or not.

Will look into it when I get back to the documentation side of things or we can open it up when I'm done with 2.0's RFC.

nick2432 commented 10 months ago

Can I contribute to this issue? I'm new here, but I have experience contributing to many other organizations. Is there a Discord channel or any other channel where I can discuss and get assistance?