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

feat(package): export type guards #1012

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

partynikko
Copy link

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.

Comment on lines +182 to +191
export { isAudioBufferSourceNode } from './guards/audio-buffer-source-node';
export { isAudioNodeOutputConnection } from './guards/audio-node-output-connection';
export { isAudioNode } from './guards/audio-node';
export { isAudioWorkletNode } from './guards/audio-worklet-node';
export { isBiquadFilterNode } from './guards/biquad-filter-node';
export { isConstantSourceNode } from './guards/constant-source-node';
export { isDelayNode } from './guards/delay-node';
export { isGainNode } from './guards/gain-node';
export { isOscillatorNode } from './guards/oscillator-node';
export { isStereoPannerNode } from './guards/stereo-panner-node';
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I did not see this pattern in the source code, it's a neat way to re-export something without explicitly importing only to do export again.

@partynikko
Copy link
Author

partynikko commented Jun 18, 2024

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
Copy link
Owner

Hi @partynikko,

thanks a lot for making me aware of that buggy type guard. I created b6b2591 to make isOscillatorNode() return false when asked to check a BiquadFilterNode. Luckily it was only used for one specific check after isBiquadFilterNode().

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 ...
}

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

Successfully merging this pull request may close these issues.

2 participants