From cc92159b9d0dc8373dd2aa8757544176c2cdf352 Mon Sep 17 00:00:00 2001 From: Simon Oxtoby Date: Sun, 24 Mar 2024 11:43:43 +1000 Subject: [PATCH] Events and event classes cannot be state --- packages/event-reduce/src/derivation.ts | 13 +++++++++ packages/event-reduce/src/events.ts | 11 +++++++- packages/event-reduce/src/models.ts | 36 ++++++++++++++++++------- tests/DerivationTests.ts | 28 +++++++++++++++++-- tests/ModelTests.ts | 31 ++++++++++++++++++++- 5 files changed, 105 insertions(+), 14 deletions(-) diff --git a/packages/event-reduce/src/derivation.ts b/packages/event-reduce/src/derivation.ts index dc852cf..ffe1469 100644 --- a/packages/event-reduce/src/derivation.ts +++ b/packages/event-reduce/src/derivation.ts @@ -1,4 +1,6 @@ +import { EventFn, IEventClass, isEvent } from "./events"; import { log, sourceTree } from "./logging"; +import { isEventsClass } from "./models"; import { IObservable } from "./observable"; import { IObservableValue, ObservableValue, collectAccessedValues, withInnerTrackingScope } from "./observableValue"; import { Unsubscribe } from "./types"; @@ -87,6 +89,10 @@ export class Derivation extends ObservableValue implements IObservableValu currentlyRunningDerivation = previouslyRunningDerivation; } }); + + if (isEvent(value) || isEventsClass(value)) + throw new DerivedEventsError(this, value); + for (let source of newSources) this._sources.set(source, this.subscribeTo(source)); this._sourceVersion = Math.max(0, ...this.sources.map(s => s.version)); @@ -152,4 +158,11 @@ export class SideEffectInDerivationError extends Error { public derivation: IObservableValue, public sideEffect: string ) { super(`Derivation ${derivation.displayName} triggered side effect ${sideEffect}. Derivations cannot have side effects.`); } +} + +export class DerivedEventsError extends Error { + constructor( + public derivation: IObservableValue, + public value: EventFn | object + ) { super(`Derivation ${derivation.displayName} returned event or events class ${isEvent(value) ? value.displayName : value}. Events cannot be state.`); } } \ No newline at end of file diff --git a/packages/event-reduce/src/events.ts b/packages/event-reduce/src/events.ts index 6467feb..ad5536f 100644 --- a/packages/event-reduce/src/events.ts +++ b/packages/event-reduce/src/events.ts @@ -164,18 +164,26 @@ class ScopedAsyncEvent(event: Event) { Object.setPrototypeOf(eventFn, event); + eventFn[eventBrand] = true; eventFn.apply = Function.prototype.apply; Object.defineProperty(eventFn, 'displayName', { // Delegate to prototype, since already initialised Subjects' displayNames will have captured the prototype as 'this' get() { return event.displayName; }, set(value: string) { event.displayName = value; } }); - return eventFn as Event & Event['next']; + return eventFn as EventFn; function eventFn(...args: any) { return event.next.apply(eventFn, args); } } +export function isEvent(event: unknown): event is EventFn { + return typeof event == 'function' + && eventBrand in event; +} + +export type EventFn = Event & Event['next'] & { [eventBrand]: true }; + /** Logs event and ensures no other events are run at the same time. */ export function fireEvent(type: string, displayName: string, arg: any, getInfo: (() => object) | undefined, runEvent: () => void) { logEvent(type, displayName, arg, getInfo, () => { @@ -195,6 +203,7 @@ export function fireEvent(type: string, displayName: string, arg: any, getInfo: let currentlyFiringEvent = null as string | null; const anonymousEvent = '(anonymous event)'; +const eventBrand = Symbol('IsEvent'); export class ChainedEventsError extends Error { constructor( diff --git a/packages/event-reduce/src/models.ts b/packages/event-reduce/src/models.ts index 3831afe..9137f2c 100644 --- a/packages/event-reduce/src/models.ts +++ b/packages/event-reduce/src/models.ts @@ -1,6 +1,6 @@ import { changeOwnedValue } from "./cleanup"; import { Derivation, derive } from "./derivation"; -import { IEventBase } from "./events"; +import { isEvent } from "./events"; import { ObservableValue, ValueIsNotObservableError, getUnderlyingObservable, startTrackingScope } from "./observableValue"; import { Reduction, reduce } from "./reduction"; import { StringKey } from "./types"; @@ -29,10 +29,14 @@ export function model(target: T): T { } for (let value of Object.values(getObservableValues(this))) - changeOwnedValue(this, undefined, value) + changeOwnedValue(this, undefined, value); - for (let key of getStateProperties(this)) + for (let key of getStateProperties(this)) { + let value = this[key]; + if (isEvent(value) || isEventsClass(value)) + throw new EventsMarkedAsStateError(this, key); changeOwnedValue(this, undefined, this[key]); + } } } }[className]; @@ -51,7 +55,7 @@ export function derived(targetOrValuesEqual: Object | ((previous: any, next: any return valuesEqual ? decorate as PropertyDecorator - : decorate(targetOrValuesEqual, key!) as any + : decorate(targetOrValuesEqual, key!) as any function decorate(target: Object, key: string | symbol) { let property = Object.getOwnPropertyDescriptor(target, key)!; @@ -186,26 +190,38 @@ export let events = (target: T): T => { super(...args); Object.keys(this).forEach(key => { let prop = this[key]; - if (hasDisplayName(prop)) { + if (isEvent(prop)) { prop.displayName = key; prop.container = this; } }); + (this as any)[eventsClassBrand] = true; } } }[className]; } -function hasDisplayName(e: any): e is IEventBase { - return e instanceof Object - && 'displayName' in e; +export function isEventsClass(value: unknown): value is Object { + return value instanceof Object + && eventsClassBrand in value; +} + +const eventsClassBrand = Symbol('IsEventsClass'); + +export class EventsMarkedAsStateError extends Error { + constructor( + public model: object, + public property: string | symbol + ) { + super(`Model property ${String(property)} returns an event or events class, and cannot be marked as @state.`); + } } export class InvalidReducedPropertyError extends Error { constructor( public property: string | symbol ) { - super(`@reduced property '${String(property)}' can only be set to the value of a reduction`); + super(`@reduced property '${String(property)}' can only be set to the value of a reduction.`); } } @@ -213,6 +229,6 @@ export class InvalidDerivedPropertyError extends Error { constructor( public property: string | symbol ) { - super(`@derived property ${String(property)} must have a getter or be set to the value of a derivation`); + super(`@derived property ${String(property)} must have a getter or be set to the value of a derivation.`); } } \ No newline at end of file diff --git a/tests/DerivationTests.ts b/tests/DerivationTests.ts index a9a701e..7f6f9d5 100644 --- a/tests/DerivationTests.ts +++ b/tests/DerivationTests.ts @@ -1,5 +1,5 @@ -import { derive, event } from "event-reduce"; -import { SideEffectInDerivationError } from "event-reduce/lib/derivation"; +import { derive, event, events } from "event-reduce"; +import { DerivedEventsError, SideEffectInDerivationError } from "event-reduce/lib/derivation"; import { consumeLastAccessed, ObservableValue } from "event-reduce/lib/observableValue"; import { spy, stub } from "sinon"; import { describe, it, then, when } from "wattle"; @@ -75,4 +75,28 @@ describe(derive.name, () => { err.has.property('sideEffect', 'some event'); }); }); + + when("derivation returns an event", () => { + let eventValue = event('derived event'); + let sut = derive(() => sourceA.value && eventValue); + + then("accessing value throws", () => { + let err = (() => sut.value).should.throw(DerivedEventsError); + err.has.property('derivation', sut); + err.has.property('value', eventValue); + }); + }); + + when("derivation returns an events class", () => { + @events + class Events { } + let eventsValue = new Events(); + let sut = derive(() => sourceA.value && eventsValue); + + then("accesing value throws", () => { + let err = (() => sut.value).should.throw(DerivedEventsError); + err.has.property('derivation', sut); + err.has.property('value', eventsValue); + }); + }); }); \ No newline at end of file diff --git a/tests/ModelTests.ts b/tests/ModelTests.ts index 8085569..7b6b265 100644 --- a/tests/ModelTests.ts +++ b/tests/ModelTests.ts @@ -1,4 +1,5 @@ -import { asyncEvent, derive, derived, event, events, extend, model, reduce, reduced } from "event-reduce"; +import { asyncEvent, derive, derived, event, events, extend, model, reduce, reduced, state } from "event-reduce"; +import { EventsMarkedAsStateError } from "event-reduce/lib/models"; import { AccessedValueWithCommonSourceError, valueChanged } from "event-reduce/lib/observableValue"; import { describe, it, test, then, when } from "wattle"; @@ -108,6 +109,20 @@ describe("models", function () { it("throws", () => increment.should.throw(AccessedValueWithCommonSourceError)); }); + + when("event is marked as state", () => { + @model + class BadModel { + @state + event = event('bad state'); + } + + it("throws when constructed", () => { + let err = (() => new BadModel()).should.throw(EventsMarkedAsStateError); + err.has.property('model'); + err.has.property('property', 'event'); + }); + }); }); describe("events decorator", function () { @@ -127,4 +142,18 @@ describe("events decorator", function () { }); it("sets event container", () => (sut.promiseEvent as any).container.should.equal(sut)); + + when("event class is marked as state", () => { + @model + class BadModel { + @state + events = new TestEvents(); + } + + it("throws when constructed", () => { + let err = (() => new BadModel()).should.throw(EventsMarkedAsStateError); + err.has.property('model'); + err.has.property('property', 'events'); + }); + }) }); \ No newline at end of file