Skip to content
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

fix!: Individualize option types for observable factories #3782

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions .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.
4 changes: 2 additions & 2 deletions packages/mobx/__tests__/v5/base/trace.ts
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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", () => {
Expand Down
17 changes: 10 additions & 7 deletions packages/mobx/src/api/annotation.ts
Expand Up @@ -6,7 +6,7 @@ export const enum MakeResult {
Continue
}

export type Annotation = {
export type Annotation<Options = unknown> = {
annotationType_: string
make_(
adm: ObservableObjectAdministration,
Expand All @@ -20,7 +20,7 @@ export type Annotation = {
descriptor: PropertyDescriptor,
proxyTrap: boolean
): boolean | null
options_?: any
options_?: Options
}

export type AnnotationMapEntry =
Expand All @@ -34,13 +34,16 @@ export type AnnotationsMap<T, AdditionalFields extends PropertyKey> = {
[P in Exclude<keyof T, "toString">]?: AnnotationMapEntry
} & Record<AdditionalFields, AnnotationMapEntry>

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_)
)
}

Expand Down
4 changes: 2 additions & 2 deletions packages/mobx/src/api/extendobservable.ts
@@ -1,5 +1,5 @@
import {
CreateObservableOptions,
CreateObservableObjectOptions,
isObservableMap,
AnnotationsMap,
asObservableObject,
Expand All @@ -17,7 +17,7 @@ export function extendObservable<A extends Object, B extends Object>(
target: A,
properties: B,
annotations?: AnnotationsMap<B, never>,
options?: CreateObservableOptions
options?: CreateObservableObjectOptions
): A & B {
if (__DEV__) {
if (arguments.length > 4) {
Expand Down
4 changes: 2 additions & 2 deletions packages/mobx/src/api/makeObservable.ts
Expand Up @@ -2,7 +2,7 @@ import {
$mobx,
asObservableObject,
AnnotationsMap,
CreateObservableOptions,
CreateObservableObjectOptions,
ObservableObjectAdministration,
collectStoredAnnotations,
isPlainObject,
Expand All @@ -22,7 +22,7 @@ import {
// Fixes: https://github.com/mobxjs/mobx/issues/2325#issuecomment-691070022
type NoInfer<T> = [T][T extends any ? 0 : never]

type MakeObservableOptions = Omit<CreateObservableOptions, "proxy">
type MakeObservableOptions = Omit<CreateObservableObjectOptions, "proxy">

export function makeObservable<T extends object, AdditionalKeys extends PropertyKey = never>(
target: T,
Expand Down
225 changes: 136 additions & 89 deletions packages/mobx/src/api/observable.ts
Expand Up @@ -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<any>
deep?: boolean
defaultDecorator?: Annotation
}

export type ComparableOption<T> = {
equals?: IEqualsComparer<T>
}

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)
Expand All @@ -73,29 +71,33 @@ const observableStructAnnotation = createObservableAnnotation(OBSERVABLE_STRUCT,
})
const observableDecoratorAnnotation = createDecoratorAnnotation(observableAnnotation)

export function getEnhancerFromOptions(options: CreateObservableOptions): IEnhancer<any> {
export function getEnhancerFromOptions(options: EnhancerOption): IEnhancer<any> {
return options.deep === true
? deepEnhancer
: options.deep === false
? referenceEnhancer
: 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<any> {
return !annotation ? deepEnhancer : annotation.options_?.enhancer ?? deepEnhancer
export function getEnhancerFromAnnotation<T>(annotation?: Annotation): IEnhancer<T> {
return (
(annotation?.options_ as { enhancer: IEnhancer<T> } | 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<T>(
v: T,
arg2?: string | number | symbol,
arg3?: CreateObservableObjectOptions
) {
// @observable someProp;
if (isStringish(arg2)) {
storeAnnotation(v, arg2, observableAnnotation)
Expand All @@ -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
Expand Down Expand Up @@ -137,36 +139,118 @@ function createObservable(v: any, arg2?: any, arg3?: any) {
}
assign(createObservable, observableDecoratorAnnotation)

export type CreateObservableValueOptions<T> = NameableOption & ComparableOption<T> & EnhancerOption

export interface IObservableValueFactory {
<T>(value: T, options?: CreateObservableOptions): IObservableValue<T>
<T>(value?: T, options?: CreateObservableOptions): IObservableValue<T | undefined>
<T>(value: T, options?: CreateObservableValueOptions<T>): IObservableValue<T>
<T>(value?: T, options?: CreateObservableValueOptions<T>): IObservableValue<T | undefined>
}

export interface IObservableFactory extends Annotation, PropertyDecorator {
<T = any>(value: T[], options?: CreateObservableOptions): IObservableArray<T>
<T = any>(value: Set<T>, options?: CreateObservableOptions): ObservableSet<T>
<K = any, V = any>(value: Map<K, V>, options?: CreateObservableOptions): ObservableMap<K, V>
<T extends object>(
value: T,
decorators?: AnnotationsMap<T, never>,
options?: CreateObservableOptions
): T
const valueFactory: IObservableValueFactory = <T>(
value: T,
options?: CreateObservableValueOptions<T>
) => {
const { name, equals, ...rest } = options ?? {}

box: IObservableValueFactory
array: <T = any>(initialValues?: T[], options?: CreateObservableOptions) => IObservableArray<T>
set: <T = any>(
return new ObservableValue(value, getEnhancerFromOptions(rest), name, true, equals)
}

export type CreateObservableArrayOptions = NameableOption & ProxyOption & EnhancerOption

export interface IObservableArrayFactory {
<T>(initialValues?: T[], options?: CreateObservableArrayOptions): IObservableArray<T>
}

const arrayFactory: IObservableArrayFactory = <T>(
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 {
<T>(
initialValues?: IObservableSetInitialValues<T>,
options?: CreateObservableOptions
) => ObservableSet<T>
map: <K = any, V = any>(
options?: CreateObservableSetOptions
): ObservableSet<T>
}

const setFactory: IObservableSetFactory = <T>(
initialValues?: IObservableSetInitialValues<T>,
options?: CreateObservableSetOptions
) => {
const { name, ...rest } = options ?? {}

return new ObservableSet<T>(initialValues, getEnhancerFromOptions(rest), name)
}

export type CreateObservableMapOptions = NameableOption & EnhancerOption

export interface IObservableMapFactory {
<K, V>(
initialValues?: IObservableMapInitialValues<K, V>,
options?: CreateObservableOptions
) => ObservableMap<K, V>
object: <T = any>(
options?: CreateObservableMapOptions
): ObservableMap<K, V>
}

const mapFactory: IObservableMapFactory = <K, V>(
initialValues?: IObservableMapInitialValues<K, V>,
options?: CreateObservableMapOptions
) => {
const { name, ...rest } = options ?? {}

return new ObservableMap<K, V>(initialValues, getEnhancerFromOptions(rest), name)
}

export type CreateObservableObjectOptions = NameableOption &
ProxyOption &
EnhancerOption &
AutoBindOption

export interface IObservableObjectFactory {
<T extends object>(
props: T,
decorators?: AnnotationsMap<T, never>,
options?: CreateObservableOptions
) => T
options?: CreateObservableObjectOptions
): T
}

const objectFactory: IObservableObjectFactory = <T extends object>(
props: T,
decorators?: AnnotationsMap<T, never>,
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.
Expand All @@ -181,52 +265,15 @@ export interface IObservableFactory extends Annotation, PropertyDecorator {
}

const observableFactories: IObservableFactory = {
box<T = any>(value: T, options?: CreateObservableOptions): IObservableValue<T> {
const o = asCreateObservableOptions(options)
return new ObservableValue(value, getEnhancerFromOptions(o), o.name, true, o.equals)
},
array<T = any>(initialValues?: T[], options?: CreateObservableOptions): IObservableArray<T> {
const o = asCreateObservableOptions(options)
return (
globalState.useProxies === false || o.proxy === false
? createLegacyArray
: createObservableArray
)(initialValues, getEnhancerFromOptions(o), o.name)
},
map<K = any, V = any>(
initialValues?: IObservableMapInitialValues<K, V>,
options?: CreateObservableOptions
): ObservableMap<K, V> {
const o = asCreateObservableOptions(options)
return new ObservableMap<K, V>(initialValues, getEnhancerFromOptions(o), o.name)
},
set<T = any>(
initialValues?: IObservableSetInitialValues<T>,
options?: CreateObservableOptions
): ObservableSet<T> {
const o = asCreateObservableOptions(options)
return new ObservableSet<T>(initialValues, getEnhancerFromOptions(o), o.name)
},
object<T extends object = any>(
props: T,
decorators?: AnnotationsMap<T, never>,
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)