Skip to content

Conversation

@kixelated
Copy link
Collaborator

A bulk way of checking if signals are null.

@kixelated kixelated requested a review from jdreetz January 29, 2026 17:27
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 29, 2026

Walkthrough

This pull request introduces a new getAll method to the Effect class in js/signals/src/index.ts and refactors five existing files to use it. The getAll method accepts an array of Getter signals and returns a tuple of their non-nullable values, or undefined if any signal is null or undefined. The refactored files in js/hang/src/ (audio encoder, broadcast, video encoder, audio source, and broadcast) replace sequential individual effect.get calls with single consolidated effect.getAll calls, consolidating retrieval logic and early-return checks.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: introducing a getAll method to reduce boilerplate code when checking multiple signals.
Description check ✅ Passed The description directly relates to the changeset, explaining the purpose of the getAll mechanism as a bulk way to check if signals are null.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

serve(track: Moq.Track, effect: Effect): void {
if (!effect.get(this.enabled)) return;

const source = effect.get(this.source);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wasn't being used, oops? getAll can catch unused variables.


const config = effect.get(this.#config);
if (!config) return;
const values = effect.getAll([this.#worklet, this.#config]);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we make this check for truthy instead of null/undefined, this can also check enabled. I'm not sure which to do.

Copy link
Collaborator

@jdreetz jdreetz left a comment

Choose a reason for hiding this comment

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

Have not tested, but looks great!


const active = effect.get(this.active);
if (!active) return;
const values = effect.getAll([this.catalog, this.broadcast, this.config, this.active]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

🤌

@kixelated kixelated merged commit acac302 into main Jan 30, 2026
1 check passed
@kixelated kixelated deleted the get-all branch January 30, 2026 11:11
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.

3 participants