Skip to content

Commit

Permalink
Detect indirect effect cycles
Browse files Browse the repository at this point in the history
  • Loading branch information
jviide committed Oct 6, 2022
1 parent 34dd5c3 commit 20bd595
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 8 deletions.
11 changes: 3 additions & 8 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,7 @@ function endBatch() {
effect._nextBatchedEffect = undefined;
effect._flags &= ~NOTIFIED;

if (
!(effect._flags & DISPOSED) &&
effect._flags & OUTDATED &&
needsToRecompute(effect)
) {
if (!(effect._flags & DISPOSED) && needsToRecompute(effect)) {
try {
effect._callback();
} catch (err) {
Expand Down Expand Up @@ -619,7 +615,7 @@ function Effect(this: Effect, compute: () => void) {
this._cleanup = undefined;
this._sources = undefined;
this._nextBatchedEffect = undefined;
this._flags = OUTDATED | TRACKING;
this._flags = TRACKING;
}

Effect.prototype._callback = function () {
Expand All @@ -643,15 +639,14 @@ Effect.prototype._start = function () {
prepareSources(this);

/*@__INLINE__**/ startBatch();
this._flags &= ~OUTDATED;
const prevContext = evalContext;
evalContext = this;
return endEffect.bind(this, prevContext);
};

Effect.prototype._notify = function () {
if (!(this._flags & NOTIFIED)) {
this._flags |= NOTIFIED | OUTDATED;
this._flags |= NOTIFIED;
this._nextBatchedEffect = batchedEffect;
batchedEffect = this;
}
Expand Down
22 changes: 22 additions & 0 deletions packages/core/test/signal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,28 @@ describe("effect()", () => {
expect(fn).to.throw(/Cycle detected/);
});

it("should throw on indirect cycles", () => {
const a = signal(0);
let i = 0;

const c = computed(() => {
a.value;
a.value = NaN;
return NaN;
});

const fn = () =>
effect(() => {
// Prevent test suite from spinning if limit is not hit
if (i++ > 200) {
throw new Error("test failed");
}
c.value;
});

expect(fn).to.throw(/Cycle detected/);
});

it("should allow disposing the effect multiple times", () => {
const dispose = effect(() => undefined);
dispose();
Expand Down

0 comments on commit 20bd595

Please sign in to comment.