From 7b3dc806a01440eae37e90ff0c76c6e4e9edc338 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Wed, 27 Sep 2023 13:34:32 -0400 Subject: [PATCH] fix!: Individualize option types for observable factories BREAKING CHANGE: Removed unused types and change the signature of some factory functions Signed-off-by: Sebastian Malton --- .changeset/tall-carrots-carry.md | 11 + packages/mobx/__tests__/v5/base/trace.ts | 4 +- packages/mobx/src/api/annotation.ts | 17 +- packages/mobx/src/api/extendobservable.ts | 4 +- packages/mobx/src/api/makeObservable.ts | 4 +- packages/mobx/src/api/observable.ts | 225 ++++++++++++-------- packages/mobx/src/mobx.ts | 15 +- packages/mobx/src/types/actionannotation.ts | 13 +- packages/mobx/src/types/dynamicobject.ts | 4 +- packages/mobx/src/types/modifiers.ts | 31 +-- packages/mobx/src/types/observableobject.ts | 6 +- packages/mobx/src/utils/utils.ts | 4 +- 12 files changed, 214 insertions(+), 124 deletions(-) create mode 100644 .changeset/tall-carrots-carry.md diff --git a/.changeset/tall-carrots-carry.md b/.changeset/tall-carrots-carry.md new file mode 100644 index 000000000..ba86823cf --- /dev/null +++ b/.changeset/tall-carrots-carry.md @@ -0,0 +1,11 @@ +--- +"mobx": major +--- + +Clarify which options are actually supported in each observable factory function. + +This change was made as a way to reduce the cognitive load on users while specifying the options when calling observable.box(...), etc... + +A user might be confused why some options don't seem to do anything. + +To update user code, the now statically disallowed options should merely be removed, they weren't used anyway. diff --git a/packages/mobx/__tests__/v5/base/trace.ts b/packages/mobx/__tests__/v5/base/trace.ts index 5055fe8a5..223b65898 100644 --- a/packages/mobx/__tests__/v5/base/trace.ts +++ b/packages/mobx/__tests__/v5/base/trace.ts @@ -65,7 +65,7 @@ describe("trace", () => { "[mobx.trace] Computed value 'x.fullname' was suspended and it will recompute on the next access." ]) - expect(expectedLogCalls).toEqual(consoleLogSpy.mock.calls) + expect(consoleLogSpy.mock.calls).toEqual(expectedLogCalls) }) test("Log only if derivation is actually about to re-run #2859", () => { @@ -121,7 +121,7 @@ describe("trace", () => { "[mobx.trace] Computed value 'x.fooIsGreaterThan5' was suspended and it will recompute on the next access." ]) - expect(expectedLogCalls).toEqual(consoleLogSpy.mock.calls) + expect(consoleLogSpy.mock.calls).toEqual(expectedLogCalls) }) test("1850", () => { diff --git a/packages/mobx/src/api/annotation.ts b/packages/mobx/src/api/annotation.ts index 87a9ea607..b2267c614 100644 --- a/packages/mobx/src/api/annotation.ts +++ b/packages/mobx/src/api/annotation.ts @@ -6,7 +6,7 @@ export const enum MakeResult { Continue } -export type Annotation = { +export type Annotation = { annotationType_: string make_( adm: ObservableObjectAdministration, @@ -20,7 +20,7 @@ export type Annotation = { descriptor: PropertyDescriptor, proxyTrap: boolean ): boolean | null - options_?: any + options_?: Options } export type AnnotationMapEntry = @@ -34,13 +34,16 @@ export type AnnotationsMap = { [P in Exclude]?: AnnotationMapEntry } & Record -export function isAnnotation(thing: any) { +export function isAnnotation(thing: unknown): thing is Annotation { + if (!((typeof thing === "object" && thing) || typeof thing === "function")) { + return false + } + + const t = thing as Annotation + return ( // Can be function - thing instanceof Object && - typeof thing.annotationType_ === "string" && - isFunction(thing.make_) && - isFunction(thing.extend_) + typeof t.annotationType_ === "string" && isFunction(t.make_) && isFunction(t.extend_) ) } diff --git a/packages/mobx/src/api/extendobservable.ts b/packages/mobx/src/api/extendobservable.ts index 44a81f916..529246130 100644 --- a/packages/mobx/src/api/extendobservable.ts +++ b/packages/mobx/src/api/extendobservable.ts @@ -1,5 +1,5 @@ import { - CreateObservableOptions, + CreateObservableObjectOptions, isObservableMap, AnnotationsMap, asObservableObject, @@ -17,7 +17,7 @@ export function extendObservable( target: A, properties: B, annotations?: AnnotationsMap, - options?: CreateObservableOptions + options?: CreateObservableObjectOptions ): A & B { if (__DEV__) { if (arguments.length > 4) { diff --git a/packages/mobx/src/api/makeObservable.ts b/packages/mobx/src/api/makeObservable.ts index 156161d24..cd55e96a4 100644 --- a/packages/mobx/src/api/makeObservable.ts +++ b/packages/mobx/src/api/makeObservable.ts @@ -2,7 +2,7 @@ import { $mobx, asObservableObject, AnnotationsMap, - CreateObservableOptions, + CreateObservableObjectOptions, ObservableObjectAdministration, collectStoredAnnotations, isPlainObject, @@ -22,7 +22,7 @@ import { // Fixes: https://github.com/mobxjs/mobx/issues/2325#issuecomment-691070022 type NoInfer = [T][T extends any ? 0 : never] -type MakeObservableOptions = Omit +type MakeObservableOptions = Omit export function makeObservable( target: T, diff --git a/packages/mobx/src/api/observable.ts b/packages/mobx/src/api/observable.ts index 01a5d119a..ef39273a9 100644 --- a/packages/mobx/src/api/observable.ts +++ b/packages/mobx/src/api/observable.ts @@ -38,27 +38,25 @@ export const OBSERVABLE_REF = "observable.ref" export const OBSERVABLE_SHALLOW = "observable.shallow" export const OBSERVABLE_STRUCT = "observable.struct" -export type CreateObservableOptions = { +export type NameableOption = { name?: string - equals?: IEqualsComparer - deep?: boolean - defaultDecorator?: Annotation +} + +export type ComparableOption = { + equals?: IEqualsComparer +} + +export type ProxyOption = { proxy?: boolean - autoBind?: boolean } -// Predefined bags of create observable options, to avoid allocating temporarily option objects -// in the majority of cases -export const defaultCreateObservableOptions: CreateObservableOptions = { - deep: true, - name: undefined, - defaultDecorator: undefined, - proxy: true +export type AutoBindOption = { + autoBind?: boolean } -Object.freeze(defaultCreateObservableOptions) -export function asCreateObservableOptions(thing: any): CreateObservableOptions { - return thing || defaultCreateObservableOptions +export type EnhancerOption = { + defaultDecorator?: Annotation + deep?: boolean } const observableAnnotation = createObservableAnnotation(OBSERVABLE) @@ -73,7 +71,7 @@ const observableStructAnnotation = createObservableAnnotation(OBSERVABLE_STRUCT, }) const observableDecoratorAnnotation = createDecoratorAnnotation(observableAnnotation) -export function getEnhancerFromOptions(options: CreateObservableOptions): IEnhancer { +export function getEnhancerFromOptions(options: EnhancerOption): IEnhancer { return options.deep === true ? deepEnhancer : options.deep === false @@ -81,21 +79,25 @@ export function getEnhancerFromOptions(options: CreateObservableOptions): IEnhan : getEnhancerFromAnnotation(options.defaultDecorator) } -export function getAnnotationFromOptions( - options?: CreateObservableOptions -): Annotation | undefined { - return options ? options.defaultDecorator ?? createAutoAnnotation(options) : undefined +export function getAnnotationFromOptions(options?: EnhancerOption): Annotation | undefined { + return options?.defaultDecorator ?? createAutoAnnotation(options) } -export function getEnhancerFromAnnotation(annotation?: Annotation): IEnhancer { - return !annotation ? deepEnhancer : annotation.options_?.enhancer ?? deepEnhancer +export function getEnhancerFromAnnotation(annotation?: Annotation): IEnhancer { + return ( + (annotation?.options_ as { enhancer: IEnhancer } | undefined)?.enhancer ?? deepEnhancer + ) } /** * Turns an object, array or function into a reactive structure. * @param v the value which should become observable. */ -function createObservable(v: any, arg2?: any, arg3?: any) { +function createObservable( + v: T, + arg2?: string | number | symbol, + arg3?: CreateObservableObjectOptions +) { // @observable someProp; if (isStringish(arg2)) { storeAnnotation(v, arg2, observableAnnotation) @@ -109,7 +111,7 @@ function createObservable(v: any, arg2?: any, arg3?: any) { // plain object if (isPlainObject(v)) { - return observable.object(v, arg2, arg3) + return observable.object(v as unknown as object, arg2, arg3) as unknown as T } // Array @@ -137,36 +139,118 @@ function createObservable(v: any, arg2?: any, arg3?: any) { } assign(createObservable, observableDecoratorAnnotation) +export type CreateObservableValueOptions = NameableOption & ComparableOption & EnhancerOption + export interface IObservableValueFactory { - (value: T, options?: CreateObservableOptions): IObservableValue - (value?: T, options?: CreateObservableOptions): IObservableValue + (value: T, options?: CreateObservableValueOptions): IObservableValue + (value?: T, options?: CreateObservableValueOptions): IObservableValue } -export interface IObservableFactory extends Annotation, PropertyDecorator { - (value: T[], options?: CreateObservableOptions): IObservableArray - (value: Set, options?: CreateObservableOptions): ObservableSet - (value: Map, options?: CreateObservableOptions): ObservableMap - ( - value: T, - decorators?: AnnotationsMap, - options?: CreateObservableOptions - ): T +const valueFactory: IObservableValueFactory = ( + value: T, + options?: CreateObservableValueOptions +) => { + const { name, equals, ...rest } = options ?? {} - box: IObservableValueFactory - array: (initialValues?: T[], options?: CreateObservableOptions) => IObservableArray - set: ( + return new ObservableValue(value, getEnhancerFromOptions(rest), name, true, equals) +} + +export type CreateObservableArrayOptions = NameableOption & ProxyOption & EnhancerOption + +export interface IObservableArrayFactory { + (initialValues?: T[], options?: CreateObservableArrayOptions): IObservableArray +} + +const arrayFactory: IObservableArrayFactory = ( + initialValues?: T[], + options?: CreateObservableArrayOptions +) => { + const { proxy = true, name, ...rest } = options ?? {} + + return ( + globalState.useProxies === false || proxy === false + ? createLegacyArray + : createObservableArray + )(initialValues, getEnhancerFromOptions(rest), name) +} + +export type CreateObservableSetOptions = NameableOption & EnhancerOption + +export interface IObservableSetFactory { + ( initialValues?: IObservableSetInitialValues, - options?: CreateObservableOptions - ) => ObservableSet - map: ( + options?: CreateObservableSetOptions + ): ObservableSet +} + +const setFactory: IObservableSetFactory = ( + initialValues?: IObservableSetInitialValues, + options?: CreateObservableSetOptions +) => { + const { name, ...rest } = options ?? {} + + return new ObservableSet(initialValues, getEnhancerFromOptions(rest), name) +} + +export type CreateObservableMapOptions = NameableOption & EnhancerOption + +export interface IObservableMapFactory { + ( initialValues?: IObservableMapInitialValues, - options?: CreateObservableOptions - ) => ObservableMap - object: ( + options?: CreateObservableMapOptions + ): ObservableMap +} + +const mapFactory: IObservableMapFactory = ( + initialValues?: IObservableMapInitialValues, + options?: CreateObservableMapOptions +) => { + const { name, ...rest } = options ?? {} + + return new ObservableMap(initialValues, getEnhancerFromOptions(rest), name) +} + +export type CreateObservableObjectOptions = NameableOption & + ProxyOption & + EnhancerOption & + AutoBindOption + +export interface IObservableObjectFactory { + ( props: T, decorators?: AnnotationsMap, - options?: CreateObservableOptions - ) => T + options?: CreateObservableObjectOptions + ): T +} + +const objectFactory: IObservableObjectFactory = ( + props: T, + decorators?: AnnotationsMap, + options?: CreateObservableObjectOptions +) => { + return initObservable(() => + extendObservable( + globalState.useProxies === false || options?.proxy === false + ? asObservableObject({}, options) + : asDynamicObservableObject({}, options), + props, + decorators + ) + ) +} + +export interface IObservableFactory + extends IObservableArrayFactory, + IObservableSetFactory, + IObservableObjectFactory, + IObservableMapFactory, + Annotation, + PropertyDecorator { + box: IObservableValueFactory + array: IObservableArrayFactory + set: IObservableSetFactory + map: IObservableMapFactory + object: IObservableObjectFactory /** * Decorator that creates an observable that only observes the references, but doesn't try to turn the assigned value into an observable.ts. @@ -181,52 +265,15 @@ export interface IObservableFactory extends Annotation, PropertyDecorator { } const observableFactories: IObservableFactory = { - box(value: T, options?: CreateObservableOptions): IObservableValue { - const o = asCreateObservableOptions(options) - return new ObservableValue(value, getEnhancerFromOptions(o), o.name, true, o.equals) - }, - array(initialValues?: T[], options?: CreateObservableOptions): IObservableArray { - const o = asCreateObservableOptions(options) - return ( - globalState.useProxies === false || o.proxy === false - ? createLegacyArray - : createObservableArray - )(initialValues, getEnhancerFromOptions(o), o.name) - }, - map( - initialValues?: IObservableMapInitialValues, - options?: CreateObservableOptions - ): ObservableMap { - const o = asCreateObservableOptions(options) - return new ObservableMap(initialValues, getEnhancerFromOptions(o), o.name) - }, - set( - initialValues?: IObservableSetInitialValues, - options?: CreateObservableOptions - ): ObservableSet { - const o = asCreateObservableOptions(options) - return new ObservableSet(initialValues, getEnhancerFromOptions(o), o.name) - }, - object( - props: T, - decorators?: AnnotationsMap, - options?: CreateObservableOptions - ): T { - return initObservable(() => - extendObservable( - globalState.useProxies === false || options?.proxy === false - ? asObservableObject({}, options) - : asDynamicObservableObject({}, options), - props, - decorators - ) - ) - }, + box: valueFactory, + array: arrayFactory, + set: setFactory, + map: mapFactory, + object: objectFactory, ref: createDecoratorAnnotation(observableRefAnnotation), shallow: createDecoratorAnnotation(observableShallowAnnotation), deep: observableDecoratorAnnotation, struct: createDecoratorAnnotation(observableStructAnnotation) } as any -// eslint-disable-next-line -export var observable: IObservableFactory = assign(createObservable, observableFactories) +export const observable: IObservableFactory = assign(createObservable, observableFactories) diff --git a/packages/mobx/src/mobx.ts b/packages/mobx/src/mobx.ts index cd7cb05af..3370d4731 100644 --- a/packages/mobx/src/mobx.ts +++ b/packages/mobx/src/mobx.ts @@ -73,7 +73,20 @@ export { transaction, observable, IObservableFactory, - CreateObservableOptions, + NameableOption, + ComparableOption, + ProxyOption, + AutoBindOption, + EnhancerOption, + CreateObservableValueOptions, + CreateObservableArrayOptions, + CreateObservableSetOptions, + CreateObservableMapOptions, + CreateObservableObjectOptions, + IObservableArrayFactory, + IObservableSetFactory, + IObservableObjectFactory, + IObservableMapFactory, computed, IComputedFactory, isObservable, diff --git a/packages/mobx/src/types/actionannotation.ts b/packages/mobx/src/types/actionannotation.ts index 7385c108b..648af2d1f 100644 --- a/packages/mobx/src/types/actionannotation.ts +++ b/packages/mobx/src/types/actionannotation.ts @@ -10,7 +10,10 @@ import { MakeResult } from "../internal" -export function createActionAnnotation(name: string, options?: object): Annotation { +export function createActionAnnotation( + name: string, + options?: Options +): Annotation { return { annotationType_: name, options_: options, @@ -72,9 +75,15 @@ function assertActionDescriptor( } } +export type ActionDescriptorOptions = { + bound?: boolean + name?: string + autoAction?: boolean +} + export function createActionDescriptor( adm: ObservableObjectAdministration, - annotation: Annotation, + annotation: Annotation, key: PropertyKey, descriptor: PropertyDescriptor, // provides ability to disable safeDescriptors for prototypes diff --git a/packages/mobx/src/types/dynamicobject.ts b/packages/mobx/src/types/dynamicobject.ts index 71598879b..44042b9e0 100644 --- a/packages/mobx/src/types/dynamicobject.ts +++ b/packages/mobx/src/types/dynamicobject.ts @@ -7,7 +7,7 @@ import { die, isStringish, globalState, - CreateObservableOptions, + CreateObservableObjectOptions, asObservableObject } from "../internal" @@ -81,7 +81,7 @@ const objectProxyTraps: ProxyHandler = { export function asDynamicObservableObject( target: any, - options?: CreateObservableOptions + options?: CreateObservableObjectOptions ): IIsObservableObject { assertProxies() target = asObservableObject(target, options) diff --git a/packages/mobx/src/types/modifiers.ts b/packages/mobx/src/types/modifiers.ts index d566113ef..e64f7c7f3 100644 --- a/packages/mobx/src/types/modifiers.ts +++ b/packages/mobx/src/types/modifiers.ts @@ -21,7 +21,7 @@ export interface IEnhancer { (newValue: T, oldValue: T | undefined, name: string): T } -export function deepEnhancer(v, _, name) { +export function deepEnhancer(v: T, _: T | undefined, name: string): T { // it is an observable already, done if (isObservable(v)) { return v @@ -29,20 +29,20 @@ export function deepEnhancer(v, _, name) { // something that can be converted and mutated? if (Array.isArray(v)) { - return observable.array(v, { name }) + return observable.array(v, { name }) as unknown as T } if (isPlainObject(v)) { - return observable.object(v, undefined, { name }) + return observable.object(v as unknown as object, undefined, { name }) as unknown as T } if (isES6Map(v)) { - return observable.map(v, { name }) + return observable.map(v, { name }) as unknown as T } if (isES6Set(v)) { - return observable.set(v, { name }) + return observable.set(v, { name }) as unknown as T } if (typeof v === "function" && !isAction(v) && !isFlow(v)) { if (isGenerator(v)) { - return flow(v) + return flow(v) as unknown as T } else { return autoAction(name, v) } @@ -50,7 +50,7 @@ export function deepEnhancer(v, _, name) { return v } -export function shallowEnhancer(v, _, name): any { +export function shallowEnhancer(v: T, _: T, name: string): T { if (v === undefined || v === null) { return v } @@ -58,16 +58,19 @@ export function shallowEnhancer(v, _, name): any { return v } if (Array.isArray(v)) { - return observable.array(v, { name, deep: false }) + return observable.array(v, { name, deep: false }) as unknown as T } if (isPlainObject(v)) { - return observable.object(v, undefined, { name, deep: false }) + return observable.object(v as unknown as object, undefined, { + name, + deep: false + }) as unknown as T } if (isES6Map(v)) { - return observable.map(v, { name, deep: false }) + return observable.map(v, { name, deep: false }) as unknown as T } if (isES6Set(v)) { - return observable.set(v, { name, deep: false }) + return observable.set(v, { name, deep: false }) as unknown as T } if (__DEV__) { @@ -75,14 +78,16 @@ export function shallowEnhancer(v, _, name): any { "The shallow modifier / decorator can only used in combination with arrays, objects, maps and sets" ) } + + return v } -export function referenceEnhancer(newValue?) { +export function referenceEnhancer(newValue: T): T { // never turn into an observable return newValue } -export function refStructEnhancer(v, oldValue): any { +export function refStructEnhancer(v: T, oldValue: T): T { if (__DEV__ && isObservable(v)) { die(`observable.struct should not be used with observable values`) } diff --git a/packages/mobx/src/types/observableobject.ts b/packages/mobx/src/types/observableobject.ts index 129b5fbff..bbf759e27 100644 --- a/packages/mobx/src/types/observableobject.ts +++ b/packages/mobx/src/types/observableobject.ts @@ -1,5 +1,4 @@ import { - CreateObservableOptions, getAnnotationFromOptions, propagateChanged, isAnnotation, @@ -47,7 +46,8 @@ import { getDebugName, objectPrototype, MakeResult, - checkIfStateModificationsAreAllowed + checkIfStateModificationsAreAllowed, + CreateObservableObjectOptions } from "../internal" const descriptorCache = Object.create(null) @@ -652,7 +652,7 @@ export interface IIsObservableObject { export function asObservableObject( target: any, - options?: CreateObservableOptions + options?: CreateObservableObjectOptions ): IIsObservableObject { if (__DEV__ && options && isObservableObject(target)) { die(`Options can't be provided for already observable objects.`) diff --git a/packages/mobx/src/utils/utils.ts b/packages/mobx/src/utils/utils.ts index fc2760b04..e31a391f7 100644 --- a/packages/mobx/src/utils/utils.ts +++ b/packages/mobx/src/utils/utils.ts @@ -97,7 +97,9 @@ export function isPlainObject(value: any) { } // https://stackoverflow.com/a/37865170 -export function isGenerator(obj: any): boolean { +export function isGenerator( + obj: any +): obj is (...args: any[]) => Generator { const constructor = obj?.constructor if (!constructor) { return false