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

Missing typings #3

Closed
kjhollen opened this issue Jul 10, 2018 · 10 comments
Closed

Missing typings #3

kjhollen opened this issue Jul 10, 2018 · 10 comments

Comments

@kjhollen
Copy link

(copying over from processing/p5.js#3078 — thanks @TeemuKoivisto!)

As the title suggests using p5.js with TypeScript results in compiler throwing errors about missing typings for various constants: Cannot find name 'COLOR_MODE'.. From what I've gathered those constants are defined in p5.global-mode.d.ts yet as I recall I couldn't make it work even by importing global-typings instead of regular p5.d.ts.

I've avoided the issue by mocking p5 with an any-value which is a sorry hack but I didn't want to invest too much time in this. Similar issue was referenced in processing/p5.js#2605

@Zalastax
Copy link
Member

Zalastax commented Jul 10, 2018

I think this might be fixed already, but I can't check this at the moment. @TeemuKoivisto please try out the typings from SNAPSHOT!
If your code is public I'd love to hear about it in #1 😃

@TeemuKoivisto
Copy link

@Zalastax Hi and thanks for the swift response. I copied the index.d.ts as my p5.d.ts but it was missing default export: node_modules/p5/lib/p5"' has no default export. I tried also importing just { p5 } but again: node_modules/p5/lib/p5"' has no exported member 'p5'.

Otherwise yes the constants work and the amp-method is fixed. I'm not sure what is the correct behavior of SoundFile's constructor as it requires a string or any[] as path yet in the examples it had no arguments https://p5js.org/examples/sound-record-save-audio.html and it does work without any arguments.

There was also a problem with p5.AudioIn interface which was missing quite a lot properties that I declared locally:

export interface IAudioIn extends p5.AudioIn {
  amplitude: p5.Amplitude
  currentSource: null // or something
  enabled: boolean
  input: GainNode
  mediaStream: MediaStreamAudioSourceNode
  output: GainNode
  stream: MediaStream
}

Aaand also since I'm here going off on a rant I noticed that the only way of saving audio is with p.saveAudio which I think is bit frustrating as now I need p5-object stored in my store. Using SoundRecorder would be so much easier with: this.recorder.saveAudio but oh well.

Regards to my project it is not nearly as ready as I'd like it to be 😅. Looks shabby and is but a curious experiment with pitch detection (that actually is not as good as I thought it would be).

@Zalastax
Copy link
Member

p5 doesn't use default exports if I reckon correctly. Instead, use:

import * as p5 from "p5";

It's great that you're working with the sound part!
Since this is all so new, no one (that I know of) has had the time to figure out how to set up a TypeScript project that uses p5 and its optional libraries. If/when you have a project that compiles, uses our typings, and runs in the browser I'd love to see it!

Regarding APIs, this repo mirrors the documentation of the official repo. If interfaces are missing: go update the documentation (we can figure out what to fix in an issue in this repo)! 😄

Where would you say you are stuck on using p5 and TS (if you are stuck, that is)?

@TeemuKoivisto
Copy link

Sorry for the late reply, I'm on vacation so I'm taking a break kind of from coding.

But yeah ups forgot about that syntax. All right that works.

Hmm I haven't faced anything too insurmountable yet although the syntax for using p5 is a bit strange. I understand it's quite similar to how d3 works and that with React which I'm using you don't really want to use d3 or p5 through React as in rendering and such (I think?). Would probably make it way too slow.

How I figured you should plug-in p5 to your React app is with a React Component -wrapper around a div or something into which p5 canvas is drawn as in react-p5-wrapper. That's kinda unintuitive or at least bit hard to set up. Truth to be told I'm not even 100% how it works. Or how props are passed down to the sketch-function where the p5 magic happens.

Then with my canvas it's also a bit peculiar how p5 is used as a local global object to create everything. Singleton pattern is a bit of a throwback to the old JS days.. =) So here for example I have my canvas with some logic about drawing frequencies and current pitch. https://github.com/TeemuKoivisto/pitch-detection/blob/master/src/p5/sketch.ts

To my eye it's a bit tangled and I haven't figured out yet how I'm going to break it into parts if it expands. I guess by first moving big methods to their responsive files but yeah having a global object that's used for (almost) everything is a bit cumbersome and impractical. Also binding directly to random p5 lifecycle methods as in my example say setup or draw is really not what I'd want. Rather I'd want to achieve the same thing directly through inheritance (like extending a React component) or by some other composition.

I'd enjoy that setting up p5 with TS (and React in that matter) was easy. Currently it's not (even if omitting breaking typings). Using a single function with p5 as a parameter and then binding to it with p.draw etc. doesn't feel right. Also the behaviour of adding say p5.sound is weird (you just add p5.sound import to the file?) without direct imports. If the types get fixed I'm already quite happy so it's not bad but this is how I feel.

Hmm that became long and sprawling. Sorry!

@Zalastax
Copy link
Member

Zalastax commented Jul 23, 2018

Your project and comments helped me set up examples/webpack. Thanks!
I'll soon look into usage with p5.dom and p5.sound. After that I'll make a minimal React example (unless someone gets to it before me).
Both of those are covered in other issues already. Do you have any other actionable feedback/issues or should we close this one?

@TeemuKoivisto
Copy link

Hmm yeah types work with the typings from the SNAPSHOT. Just need the p5.sound fixes too and I can throw away my local typings. I'm looking forward to seeing your React-example! So sure you can close this.

@Zalastax
Copy link
Member

Then I see one actionable thing here: fix the YUIDoc comments for p5.sound!
Just add properties on IAudioIn?

@arjpar
Copy link

arjpar commented Jul 26, 2018

I'm getting errors too. But I'm using a reference path in my typescript file. The global.d.ts file comes up with errors. I tried using the ones on the SNAPSHOT branch, but it doesn't recognize the p5 type.

@Zalastax
Copy link
Member

@K06RA can you try out the new SNAPSHOT definitions? You can download them from SNAPSHOT.zip.
Point a reference to the file, like this: global-test.ts

@Zalastax
Copy link
Member

@TeemuKoivisto I've added a React example now!
I couldn't really come up with any improvements to make so all the awkwardness you pointed out is still there.
The "local global object" is just how p5 works in instance mode so I don't think there's anything we can improve here. I suppose you could drive for moving functions off the p5 instance and expose them as normal functions, but that would have to be done in the main repository.
p5 sound fixes are comming: processing/p5.js-sound#310.
Thanks again for creating a project that helped me a lot with the examples!

Feel free to open a new issue, but I'm closing this one since it's a bit messy and I think everything is resolved.

@dcsan dcsan mentioned this issue Aug 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants