Truthy

[kixelated]

getAll checks for truthy instead of being lame and checking for null.

[coderabbitai]

Walkthrough

This pull request refactors multiple modules in the publish and watch subsystems to consolidate value retrieval patterns. Individual effect.get calls retrieving the enabled signal and other dependencies are replaced with single effect.getAll calls that fetch multiple values simultaneously. The enabled flag is retrieved but typically ignored via placeholder destructuring. Explicit guards checking the enabled state before proceeding are removed, changing control flow to depend on whether combined value retrieval succeeds. Additionally, the Effect.getAll method signature is updated to return Truthy types instead of NonNullable, and new type aliases for Falsy and Truthy are introduced to support this change. The signals package version is bumped to 0.1.2.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Truthy' directly describes the main change: updating getAll to check for truthy values instead of null checks.
Description check	✅ Passed	The description accurately explains that getAll now checks for truthy instead of null, which matches the changeset's core modification.
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

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch truthy

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@js/hang/src/publish/broadcast.ts`:
- Around line 60-61: The current check only returns when path === undefined, but
effect.get(this.path) can now return null and that null may reach
connection.publish; update the guard around the path obtained from
effect.get(this.path) (the local variable named path in the method where you
call connection.publish) to treat both null and undefined as missing (e.g., use
a nullish check like path == null or path === null || path === undefined) and
return early to preserve the previous non-null behavior before calling
connection.publish.

In `@js/signals/src/index.ts`:
- Around line 421-432: The runtime check in getAll uses `if (!value)` which
wrongly treats `0`, `NaN`, `0n`, etc. as falsy and narrows values more than the
Falsy/Truthy types; change the check to only reject null/undefined (e.g. `if
(value === undefined || value === null) return undefined;`) so GetterType/Truthy
semantics are preserved; update the check in getAll (and any related helper)
instead of using `!value`.

🧹 Nitpick comments (1)

js/hang/src/publish/source/file.ts (1)

21-24: Put enabled first in getAll to short-circuit earlier.

With the current order, file is subscribed even when disabled, so file changes can rerun the effect while decoding is off. Reordering avoids that extra churn.

♻️ Suggested reorder

-			const values = effect.getAll([this.file, this.enabled]);
+			const values = effect.getAll([this.enabled, this.file]);
 			if (!values) return;
-			const [file] = values;
+			const [_, file] = values;

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Guard against null path values explicitly.

Now that path is no longer part of getAll, null would slip past path === undefined and can reach connection.publish, which could throw. Consider guarding both null and undefined to preserve the previous “non-null” behavior.

🔧 Suggested fix

-		const path = effect.get(this.path);
-		if (path === undefined) return;
+		const path = effect.get(this.path);
+		if (path == null) return;

🤖 Prompt for AI Agents

In `@js/hang/src/publish/broadcast.ts` around lines 60 - 61, The current check
only returns when path === undefined, but effect.get(this.path) can now return
null and that null may reach connection.publish; update the guard around the
path obtained from effect.get(this.path) (the local variable named path in the
method where you call connection.publish) to treat both null and undefined as
missing (e.g., use a nullish check like path == null or path === null || path
=== undefined) and return early to preserve the previous non-null behavior
before calling connection.publish.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cd js/signals && find . -name "*.ts" -o -name "*.tsx" | head -20

Repository: moq-dev/moq

Length of output: 138

---

🏁 Script executed:

cd js/signals && cat -n src/index.ts | sed -n '415,440p'

Repository: moq-dev/moq

Length of output: 1022

---

🏁 Script executed:

cd js/signals && grep -n "type Falsy\|type Truthy" src/index.ts | head -20

Repository: moq-dev/moq

Length of output: 146

---

🏁 Script executed:

cd js/signals && rg "Falsy|Truthy" --type ts -B 2 -A 2

Repository: moq-dev/moq

Length of output: 786

---

Runtime truthiness is stricter than the Falsy type definition.

!value also treats NaN and 0n as falsy, which aren't included in Falsy/Truthy. If only the explicit falsy values should be excluded, use explicit comparisons:

Suggested fix

-			if (!value) return undefined;
+			if (value === false || value === 0 || value === "" || value === null || value === undefined) {
+				return undefined;
+			}

Alternatively, document that the function also excludes NaN and 0n.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Get the values of multiple signals, returning undefined if any are falsy.
	getAll<S extends readonly Getter<unknown>[]>(
	signals: [...S],
	): { [K in keyof S]: NonNullable<GetterType<S[K]>> } | undefined {
	): { [K in keyof S]: Truthy<GetterType<S[K]>> } | undefined {
	const values: unknown[] = [];
	for (const signal of signals) {
	const value = this.get(signal);
	if (value == null) return undefined;
	if (!value) return undefined;
	values.push(value);
	}
	return values as { [K in keyof S]: NonNullable<GetterType<S[K]>> };
	return values as { [K in keyof S]: Truthy<GetterType<S[K]>> };
	}
	// Get the values of multiple signals, returning undefined if any are falsy.
	getAll<S extends readonly Getter<unknown>[]>(
	signals: [...S],
	): { [K in keyof S]: Truthy<GetterType<S[K]>> } | undefined {
	const values: unknown[] = [];
	for (const signal of signals) {
	const value = this.get(signal);
	if (value === false || value === 0 || value === "" || value === null || value === undefined) {
	return undefined;
	}
	values.push(value);
	}
	return values as { [K in keyof S]: Truthy<GetterType<S[K]>> };
	}

🤖 Prompt for AI Agents

In `@js/signals/src/index.ts` around lines 421 - 432, The runtime check in getAll
uses `if (!value)` which wrongly treats `0`, `NaN`, `0n`, etc. as falsy and
narrows values more than the Falsy/Truthy types; change the check to only reject
null/undefined (e.g. `if (value === undefined || value === null) return
undefined;`) so GetterType/Truthy semantics are preserved; update the check in
getAll (and any related helper) instead of using `!value`.