-
Notifications
You must be signed in to change notification settings - Fork 141
Truthy #895
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
Truthy #895
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -158,6 +158,10 @@ export class Signal<T> implements Getter<T>, Setter<T> { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type SetterType<S> = S extends Setter<infer T> ? T : never; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type GetterType<G> = G extends Getter<infer T> ? T : never; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Excludes common falsy values from a type | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type Falsy = false | 0 | "" | null | undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type Truthy<T> = Exclude<T, Falsy>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // TODO Make this a single instance of an Effect, so close() can work correctly from async code. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export class Effect { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Sanity check to make sure roots are being disposed on dev. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -414,17 +418,17 @@ export class Effect { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.#dispose.push(() => effect.close()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Get the values of multiple signals, returning undefined if any are null/undefined. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 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]>> }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+421
to
432
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: cd js/signals && find . -name "*.ts" -o -name "*.tsx" | head -20Repository: 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 -20Repository: moq-dev/moq Length of output: 146 🏁 Script executed: cd js/signals && rg "Falsy|Truthy" --type ts -B 2 -A 2Repository: moq-dev/moq Length of output: 786 Runtime truthiness is stricter than the
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 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // A helper to call a function when a signal changes. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard against
nullpath values explicitly.Now that
pathis no longer part ofgetAll,nullwould slip pastpath === undefinedand can reachconnection.publish, which could throw. Consider guarding bothnullandundefinedto preserve the previous “non-null” behavior.🔧 Suggested fix
🤖 Prompt for AI Agents