Tone.js icon indicating copy to clipboard operation
Tone.js copied to clipboard

Need adequate example (docs) or convenient interface for PolySynth options

Open anselanza opened this issue 4 years ago • 9 comments

Example or convenient interface for PolySynth options When using Typescript, I am finding it impossible to specify the correct type to pass down to React components for the "options" parameter when constructing new Tone.PolySynth.

A helpful example of exactly what type signature (with generics, etc.) to use would be useful. Alternatively, some kind of alias might be useful if the type definition ends up becoming very hard to use (lots of generics, etc.).

Any alternatives you've considered I have tried PolySynth<Synth<SynthOptions>>, for example:

const synthOptions: PolySynth<Synth<SynthOptions>> = {
  detune: 0,
  portamento: 0,
  harmonicity: 17,
  volume: -20,
  oscillator: {
// ... etc.

This type is allowed to be passed into new Tone.PolySynth (Tone.FMSynth, synthOptions) without complaint, but the TS compiler will complain about the definition of synthOptions itself:

Type 'number' is not assignable to type 'Param<"decibels">'.ts(2322)
Instrument.d.ts(27, 5): The expected type comes from property 'volume' which is declared here on type 'PolySynth<Synth<SynthOptions>>'

(and so on).

This also does not work:

type Options = ConstructorParameters<Tone.PolySynth>

... fails with:

Type 'PolySynth<Synth<SynthOptions>>' does not satisfy the constraint 'abstract new (...args: any) => any'.
  Type 'PolySynth<Synth<SynthOptions>>' provides no match for the signature 'new (...args: any): any'.ts(2344)

My only viable solution for now is to use any:

interface Props {
  synthOptions: any;
// ...

Additional context In spite of the recommendations of https://github.com/Tonejs/Tone.js/wiki/Using-Tone.js-with-React-React-Typescript-or-Vue I am not able to type options for a PolySynth instance without errors.

anselanza avatar Oct 20 '21 08:10 anselanza

I think you can simplify your typing to just const options: SynthOptions = . The generics are there so that PolySynth is can take in multiple synth types and to ensure that the options that you pass in match the synth constructor that you passed in.

tambien avatar Oct 21 '21 17:10 tambien

I think you can simplify your typing to just const options: SynthOptions = . The generics are there so that PolySynth is can take in multiple synth types and to ensure that the options that you pass in match the synth constructor that you passed in.

I'm afraid that doesn't work, unfortunately.

Screenshot from 2021-10-27 16-36-00

and...

Screenshot from 2021-10-27 16-37-22

anselanza avatar Oct 27 '21 14:10 anselanza

Your oscillator options should have either partialsCount or a partials array but not both. partialsCount is meant to limit the number of partials of one of the basic oscillator types (sine, square, etc) but not a custom partials array.

tambien avatar Oct 27 '21 17:10 tambien

You might correct about that, but commenting out partialsCount simply pushes further problems. Next, TS complains about a bunch of properties missing in oscillator, so I added these... next I get a complaint about the property harmonicity not existing. Screenshot 2021-10-28 at 13 11 30

So I guess I could keep going like this?

But why then does the example in https://tonejs.github.io/examples/fmSynth include properties that do not exist? Is the example out of date? Screenshot 2021-10-28 at 13 13 30

anselanza avatar Oct 28 '21 11:10 anselanza

For the FMSynth, you should use FMSynthOptions as the type for the options object. It should have harmonicity and the other attributes which belong to that synth.

The generic for PolySynth is meant to match the synth constructor to its corresponding options.So it'll ensure that Synth is matched with SynthOptions or AMSynth with AMSynthOptions.

tambien avatar Oct 29 '21 02:10 tambien

Right, so we get a little closer by using FMSynthOptions but still the example provided on the website would break if used as-is: Screenshot 2021-11-01 at 14 18 41

Notice that I have to add properties such as muted, volume and onstop in a few places (which seems redundant)... and... we're still getting compiler complaints!

anselanza avatar Nov 01 '21 13:11 anselanza

If you don't want to fill in all of the interface attributes, set your type to be const synthOptions: Partial<FMSynthOptions>, that'll let the TS compiler know you don't intend on using the entire interface, but just a part of it.

tambien avatar Nov 01 '21 18:11 tambien

I'm afraid that doesn't do it, either, because the oscillator property (which in turn is defined by OmitSourceOptions<ToneCustomOscillatorOptions>) still isn't satisfied as far as the compiler is concerned. Screenshot 2021-11-02 at 10 13 07

Sorry if this is tedious, and feels like a "debugging session via Issue", but I am genuinely curious what the correct/intended approach should be here. It seems like a rather common use case to want to define some options for a synth/instrument and pass these as arguments or props while enjoying the convenience of the type system checking for the required properties (and preferably only the required properties).

Is there a problem with the way the type definitions are currently defined in the library? Or do we need some clear examples that show the proper way to use the type definitions in a case such as this?

anselanza avatar Nov 02 '21 09:11 anselanza

Ok looked into this a little further and it seems like because that object type is nested it needs to not be Partial<FMSynthOptions>, but instead RecursivePartial<FMSynthOptions> so that it could also be a Partial of the oscillator object as well for example. This all seems to be correctly inferred when you simply pass the object into FMSynth, but obviously gives you some errors when you explicitly assign it like you're doing in your example.

One option which seems to work is to explicitly grab RecursivePartial from like this: import { RecursivePartial } from "tone/build/esm/core/util/Interface" and define your options object as RecursivePartial<FMSynthOptions>. That seems to work fine in my testing but obviously a little cumbersome.

If you'd like to use more builtin typescript types instead, you can grab the first argument to the constructor of FMSynth like this:

const synthOptions: ConstructorParameters<typeof FMSynth>[0] = ...

That seems to work fine with the arguments that you posted.

So the options for this specific situation aren't great currently. We could export another type called PartialFMSynthOptions defined as RecursivePartial<FMSynthOptions>. Any of these options are a little cumbersome, so open to other suggestions if you know a better way to handle types here.

tambien avatar Nov 08 '21 18:11 tambien