Skip to content

Commit

Permalink
Replace public peek method with friend-style symbol
Browse files Browse the repository at this point in the history
Peeking a value in contexts other than internal parts of the reactivity
system itself tends very strongly to produce bugs, because it decouples
consumers from the root state. (It is very, very tempting to wire your
own caching on with a "peek", rather than using caching tools composed
out of the core primitives, or to "be smarter" than the signal system.)

For an interesting background discussion from the history of Glimmer's
similar tag/signals system, see [here][github].

[github]: emberjs/rfcs#656 (comment)

This doesn't *force* us to keep that, but it sets a nice precedent and
means if we *do* introduce `peek()` as public behavior, we'll have a
clear indicator that we need to make the choice explicitly.
  • Loading branch information
chriskrycho committed Nov 20, 2022
1 parent c3e5b29 commit db5dd6a
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 3 deletions.
13 changes: 12 additions & 1 deletion src/signal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ function baseEquality<T>(oldValue: T, newValue: T) {
}

export class Signal<T = unknown> {
/**
@private This is available for internal "friend" APIs to use, but it is *not*
legal to use by consumers.
*/
export const Peek = Symbol('Peek');

#value: T;
protected isEqual: Equality<T>;
protected tag: Tag;
Expand All @@ -27,7 +33,12 @@ export class Signal<T = unknown> {
return this.#value;
}

peek() {
// Expressly *not* part of the public API: peeking a value in contexts other than internal parts
// of the reactivity system itself tends very strongly to produce bugs, because it decouples
// consumers from the root state. (It is very, very tempting to wire your own caching on with a
// "peek", rather than using caching tools composed out of the core primitives, or to "be smarter"
// than the signal system.)
[Peek](): T {
return this.#value;
}

Expand Down
4 changes: 2 additions & 2 deletions tests/signal.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect, describe, test, vi } from 'vitest';
import { createSignal } from '../src/signal';
import { createSignal, Peek } from '../src/signal';
import { createEffect } from '../src/effect';
import isEqual from 'lodash/isEqual';

Expand Down Expand Up @@ -50,7 +50,7 @@ describe('Signal', () => {
let foo = createSignal('foo');

let spy = vi.fn(() => {
foo.peek();
foo[Peek]();
});

createEffect(spy);
Expand Down

0 comments on commit db5dd6a

Please sign in to comment.