chrisguttandin / standardized-audio-context

A cross-browser wrapper for the Web Audio API which aims to closely follow the standard.
MIT License
680 stars 33 forks source link

feat(package): export type guards #1012

Open partynikko opened 5 months ago

partynikko commented 5 months ago

This PR aims to solve #1011.

By exporting type guards consumers, like myself, can use said type guards to narrow down different audio nodes for different behaviours. I am currently experimenting with creating a declarative audio renderer, hence my need to narrow down declared nodes to their instances.

I have built and verified that I can import it and use it from another project.

partynikko commented 5 months ago

I just noticed that there are false positives amongst the type guards. For example, isOscillatorNode will return true for a BiquadFilterNode because of both detune and frequency being present in the latter.

isOscillatorNode(biquadFilterNode) // true

Have not tested others yet so there might be more? Will need to investigate, if this PR is to be merged. Let me know.

chrisguttandin commented 4 months ago

Hi @partynikko,

thanks a lot for making me aware of that buggy type guard. I created https://github.com/chrisguttandin/standardized-audio-context/commit/b6b2591c7b2f2d3a3cf076205c2fe0038c9a7f8d to make isOscillatorNode() return false when asked to check a BiquadFilterNode. Luckily it was only used for one specific check after isBiquadFilterNode().

https://github.com/chrisguttandin/standardized-audio-context/blob/b6b2591c7b2f2d3a3cf076205c2fe0038c9a7f8d/src/helpers/deactivate-active-audio-node-input-connections.ts#L36

https://github.com/chrisguttandin/standardized-audio-context/blob/b6b2591c7b2f2d3a3cf076205c2fe0038c9a7f8d/src/helpers/deactivate-active-audio-node-input-connections.ts#L42

I think this bug illustrates the problem with type guards very well. They don't really check if the logic actually makes sense. I'm not sure if exposing them would be a good idea?

Would instanceof checks work for your use case?

import { OscillatorNode } from 'standardized-audio-context';

if (oscillatorNode instanceof OscillatorNode) {
    // Yeah, it's an OscillatorNode ...
}