Skip to content

@computed decorator should be lazy #4616

@pie6k

Description

@pie6k

Looking at current decorators internal implementation - it eagerly creates new ComputedValue for every decorated property for every new instance

function decorate_20223_(this: Annotation, get, context: ClassGetterDecoratorContext) {
    if (__DEV__) {
        assert20223DecoratorType(context, ["getter"])
    }
    const ann = this
    const { name: key, addInitializer } = context

    addInitializer(function () {
        const adm: ObservableObjectAdministration = asObservableObject(this)[$mobx]
        const options = {
            ...ann.options_,
            get,
            context: this
        }
        options.name ||= __DEV__
            ? `${adm.name_}.${key.toString()}`
            : `ObservableObject.${key.toString()}`
        adm.values_.set(key, new ComputedValue(options))
    })

    return function () {
        return this[$mobx].getObservablePropValue_(key)
    }
}

I think it is quite a waste of memory and performance:

  • I think it is safe to say, many of computed properties of some instances will never ever be used
  • say we have 1k instances, each have 5 @computed getters
  • say on each instance, we only ever use 2 out of those 5 during their lifetime
  • as a result - 3000 ComputedValue instances were created, while never being used
  • those 3000 took some time to create and did occupy some memory before being disposed

What, if decorator would be implemented something like in example below.

Here, we do not add addInitialize and create computed value once on the first getter use

  • if computed value is never used - ComputedValue is never created
  • as there is no addInitialize - it should have lower performance penantly eg when creating 1000s of new instances with decorators
  • AFAIK the only initial runtime cost will be similar internal prototype update during class declaration. Creating class instance will have comparable performance to creating normal, undecorated instance
import { ComputedValue } from "mobx/dist/internal";

const COMPUTED_SYMBOL = Symbol("computed");
type Key = string | symbol;
type ComputedMap = Record<Key, ComputedValue<any>>;

function initializeComputed(instance: any, key: Key, getter: () => any): ComputedValue<any> {
  // if needed - initialize our map of computed values
  const computedMap: ComputedMap = (instance[COMPUTED_SYMBOL] ??= {});

  // if needed - initialize new `ComputedValue`
  return (computedMap[key] ??= new ComputedValue({
    get: getter,
    context: instance,
  }));
}

export function $computed(get: () => any, context: ClassGetterDecoratorContext) {
  const { name: key } = context;

  return function (this: any) {
    // when used - initialize computed if needed and get its value
    return initializeComputed(this, key, get).get();
  };
}

Intended outcome:

When creating instance with @computed getter, but never using it - ComputedValue should never be created

Actual outcome:

ComputedValue is created even if we never use it

How to reproduce the issue:

On created instance, check $mobx special property - computed value is there.

Versions

6.15.0

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions