Skip to content
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.

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

Replace any/object with more accurate types #31

Open
1 task done
fal-works opened this issue Jan 10, 2021 · 6 comments
Open
1 task done

Replace any/object with more accurate types #31

fal-works opened this issue Jan 10, 2021 · 6 comments
Labels
require upstream Need upstream to update

Comments

@fal-works
Copy link
Contributor

fal-works commented Jan 10, 2021

(Versions I'm referring to: p5 v1.2.0, @types/p5 v0.9.1)

This won't be easy as the typings are automatically generated and we'll need extra work for specifying more accurate types, but at least it would be worth leaving an issue I think.

Here is a list of things that would be nice to see improved:

Event handler functions

Concrete type for event instead of object.
(Also the return types should be fixed, but maybe it's p5.js documentation issue)

Something like:

type UIEventCallback = (event?: UIEvent) => void | boolean;
type KeyboardEventCallback = (event?: KeyboardEvent) => void | boolean;
type MouseEventCallback = (event?: MouseEvent) => void | boolean;
type TouchEventCallback = (event?: TouchEvent) => void | boolean;

interface p5InstanceExtensions {
  windowResized: UIEventCallback;

  keyPressed: KeyboardEventCallback;
  // ... and so on

  mousePressed: MouseEventCallback;
  // ... and so on

  touchStarted: TouchEventCallback;
  // ... and so on
}

random(choices) and Array functions

Would be nice if we can use generics instead of any.

For random(), something like:

interface p5InstanceExtensions {
  random<T>(choices: T[]): T;
}

and for array functions:

interface p5InstanceExtensions {
  append<T>(array: T[], value: T): T[];

  // ... and so on
}

p5.Element

  • p5.Element.elt should be HTMLElement instead of any.

This property may be used frequently when playing around with DOM.

interface Element {
  elt: HTMLElement;
}

Ideally, p5.Element might also be declared with generics so that we'll have more accurate types, e.g. canvas.elt: HTMLCanvasElement.

saveFrames()

Personally I haven't used this yet, but we could add the argument type of the callback funciton.

Something like:

type Frame = {
  imageData: string;
  filename: string;
  ext: string;
};

interface p5InstanceExtensions {
  saveFrames: (filename: string, extension: string, duration: number, framerate: number, callback?: (frames: Frame[]) => void) => void;
}

Other

  • any[] in trim() and some type convertion functions
  • any in callbacks of some webgl functions
  • object types in p5.sound
  • (maybe some more tiny cases)

And of course there might also be some cases where any/object are still appropriate.

@fal-works fal-works changed the title Replace any/object with more accurate types Replace any/object with more accurate types Jan 10, 2021
@Zalastax
Copy link
Member

Sounds like good suggestions!
There are a few ways we can achieve improvements in general:

  • If the data already exists in data.json: start handling this type in a generic fashion.
  • If the data does not exist but is representable in YUIDoc: Add as YUIDoc to ps.js and then start handling it in a generic fashion.
  • If the data is not representable in YUIDoc: Add special case or invent a way to represent in YUIDoc.

Feel free to email me if you want help to get started / bounce ideas.

@fal-works
Copy link
Contributor Author

Thanks for the reply!
Seems that most of the data exist in data.json.

In the p5.js YUIDoc, it looks like several {Object}/{Array} types can be replaced with more concrete types such as {HTMLElement} or {Number[]} or something.
But maybe this will need to be discussed, and depending on the conclusion we might need to write the types manually, separated from p5.js.

Not sure if generic functions can be represented in a valid YUIDoc syntax (which seems to be possible in JSDoc with @template tags).

@Zalastax
Copy link
Member

I went ahead and asked whether YUIDoc has a similar feature: yui/yuidoc#452. If not, perhaps we can add some dummy types in p5.js, that are just aliases (if that is supported) of Object (like GenericS, GenericT...)

But maybe this will need to be discussed

Best way to see is to go ahead and contribute a pull request!

@fal-works
Copy link
Contributor Author

OK, tried to suggest: processing/p5.js#4991

@asukaminato0721
Copy link
Contributor

grep "any\[\]" -r ./types/p5/

gives a lot of results, fix/patch them will improve user experiences.

@asukaminato0721
Copy link
Contributor

Thanks for the reply! Seems that most of the data exist in data.json.

In the p5.js YUIDoc, it looks like several {Object}/{Array} types can be replaced with more concrete types such as {HTMLElement} or {Number[]} or something. But maybe this will need to be discussed, and depending on the conclusion we might need to write the types manually, separated from p5.js.

Not sure if generic functions can be represented in a valid YUIDoc syntax (which seems to be possible in JSDoc with @template tags).

any examples? I could make PR for them.

But maybe this will need to be discussed, and depending on the conclusion we might need to write the types manually, separated from p5.js.

I am doing it lol.

Not sure if generic functions can be represented in a valid YUIDoc syntax (which seems to be possible in JSDoc with @template tags).

I guess not, yui-doc is legacy now. Unless p5.js change to js-doc or something else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
require upstream Need upstream to update
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants