cellx icon indicating copy to clipboard operation
cellx copied to clipboard

isPending, getError упаковать в один метод

Open zerkalica opened this issue 7 years ago • 9 comments

Предлагаю ввести понятие метаданных атома, которые тоже являются атомом, но просачиваются сквозь computables, комбинируясь по нижеприведенной схеме. Т.е. упаковать isPending, getError в один метод getStatus(): FetchStatus. По признаку того, что это все метаданные атома. Статусы могут со временем дополняться, состояние метаданных атома не ограничивается pending/error, рефакторить будет проще.

// @flow

export type FetchStatusType = 'error' | 'pending' | 'complete'

export default class FetchStatus {
    complete: boolean
    pending: boolean
    error: ?Error

    constructor(
        type?: FetchStatusType,
        error?: ?Error
    ) {
        this.complete = type === 'complete'
        this.pending = !type || type === 'pending'
        this.error = error || null
    }

    static merge(statuses: FetchStatus[]): FetchStatus {
        const newStatus = new FetchStatus('complete')
        for (let i = 0, l = statuses.length; i < l; i++) {
            const status = statuses[i]
            if (status.pending) {
                newStatus.pending = true
                newStatus.complete = false
            }
            if (status.error) {
                newStatus.error = status.error
                newStatus.complete = false
                newStatus.pending = false
                break
            }
        }

        return newStatus
    }
}

zerkalica avatar Nov 12 '16 11:11 zerkalica

В opti-update updater управляет состоянием атома и его статусом, класс которого приведен выше. Сейчас статус - это отдельный атом, которыя я передаю в транзакцию. Этот статус я не могу неявно наследовать в computable.

console.log('update a, b, set status.pending:')
updater.transaction()
    .set(a, '2')
    .set(b, '2')
    .run({
        type: 'promise',
        atom: a,
        status: aStatus,
        fetch() {
            return Promise.resolve('3')
        }
    })

zerkalica avatar Nov 13 '16 07:11 zerkalica

У них немного по разному сделана ленивость, isPending создаёт скрытую ячейку при первом обращении к нему, а getError при первом обращении к нему в формуле, то есть при подписке. Хотелось бы "в формуле" сделать и для isPending, но похоже не получиться, если же я объединю их в одну ячейку, то потеряю "в формуле" и для getError. Плюс нельзя будет подписаться отдельно на isPending или на getError, прийдётся подписываться на оба сразу и обрабатывать лишнее, например, изменения getError когда зависимость вычисляется только из isPending.

Riim avatar Nov 14 '16 11:11 Riim

В случае fetch, pending неразрывно связан с error. Т.е. где error, там и pending требуется показать. Кода меньше получается и проще работать с такой цельной сущностью, тот же merge.

Например, в моем todomvc, с помощью компонента ServerLoadingView отображается либо индикатор, либо плашка с ошибкой.

А в TodoLayout используется ServerLoadingView.

    (!status.complete)
                ? <ServerLoadingView status={status} />
                : children
            }

На проблему можно взглянуть под другим углом. Кроме самих атомов, есть еще как бы теневое состояние атомов, который сейчас "размазано" в isPending, isError. У этого теневого состояния есть свойство просачиваться в computables, комбинируясь по особому алгоритму (FetchStatus.merge). Сейчас это у тебя уже реализовано, только в виде отдельных податомов.

Вот я и предлагаю решить задачу в общем виде, сделать точку расширения, что б сторонние библиотеки могли сами определять структуру этого теневого состояния и алгоритм его комбинации.

На мой взгляд api cellx несколько усложнен и есть тенденция к его разбуханию. isPending, getError по мне должны быть инкапсулированны в отдельный модуль.

Пример, вот все, что мой updater знает о cellx:

// @flow
interface Atom<V> {
    set(val: V): void;
    get(): V;
}

a, b, aStatus - атомы. Пример ниже обновляет a, b, ставит aStatus.pending выполняет fetch, по resolve ставит aStatus.complete и a = 3.

updater.transaction()
    .set(a, '2')
    .set(b, '2')
    .run({
        type: 'promise',
        atom: a,
        status: aStatus,
        fetch() {
            return Promise.resolve('3')
        }
    })

Все круто, но мне хотелось бы использовать механизм теневого состояния cellx. и получать aStatus из а, а не отдельный атом aStatus создавать, что бы этот aStatus протекал в computable.

Как-то так:

cellx.configure({mergeShadowState: FetchStatus.merge})
aStatus = a.createShadow(new UpdaterStatus())

zerkalica avatar Nov 14 '16 12:11 zerkalica

Добавил Cell#getStatus.

if (status.error) {
                newStatus.error = status.error
                newStatus.complete = false

завершение с ошибкой - это тоже вполне себе завершение, то есть complete должен переводиться в true.

Riim avatar Nov 17 '16 09:11 Riim

правильнее было назвать success, когда нет ошибки и не pending. Это свойство я ввел, т.к. проще логику строить.

function ErrorOrPendingComponent({status}: {status: FetchStatus}) {
    if(status.error) {
       return <Error error={status.error}/>
    }
    return <Loader/>
}

function Component({status}: {status: FetchStatus}) {
  if (!status.complete) {
    return <ErrorOrPendingComponent status={status}/>
  }

  return <div>Component</div>
}

zerkalica avatar Nov 17 '16 09:11 zerkalica

Вспомнил, я делал по аналогии со спецификацией Observable: next, error, complete. И там complete не может быть с error.

Кстати, push/fail - это насколько я понимаю, велосипед Карловского, лучше завязываться на официальные спеки, на Observer {next, error, complete}.

                observer.next("hello");
                observer.next("world");
                observer.complete();

zerkalica avatar Nov 17 '16 10:11 zerkalica

В 1.6.77 заменил error на success.

Кстати, push/fail - это насколько я понимаю, велосипед Карловского

довольно удачный велосипед, Observable - это стримовая реализация, её нейминг не всегда удачно подходит для ячеек/атомов. Например, complete звучит как-то одноразово и это подходит стримам, но для ячейки вполне нормально вызвать reap несколько раз. У меня, наверное, плохо то, что калбек называется reap, а соответствующий метод dispose. Надо добавить reap, а dispose оставить его алиасом.

Riim avatar Nov 18 '16 08:11 Riim

Добавил Cell#reap.

Riim avatar Nov 18 '16 08:11 Riim

Зачем вообще reap/dispose? разве subscribe/unsubscribe не тоже самое делают? подписался - счетичик в зависимостях +1, отписался счетчик-1, если === 0, то дергаем reap колбэк?


Ну как бы ни звучала, я на нее замахивался из-за упрощения интеграции со сторонними решениями. Хоть хреновая, но спецификация, более формальная, чем чем event-emitter. В принципе не столь важно, в opti-update я сделал обертку

Тут может не так очевидно, как с reactive-di. Вот компонент TodosView

export default function TodosView(
    {todos}: TodosViewProps,
    {theme}: TodosViewState
) {
    return <ul className={theme.wrapper}>
        {todos.items.map((todo: Todo) =>
            <li className={theme.item} key={todo.id}>
                <TodoView todo={todo}/>
            </li>
        )}
    </ul>
}

Выглядит как react, flow и typescript понимают его props как реактовый, но он не зависит от реакта, это компонент с нулевыми зависимостями:

function TodosView(_ref, _ref2, __h) {
    var todos = _ref.todos;
    var theme = _ref2.theme;

    return __h(
        'ul',
        { className: theme.wrapper },
        todos.items.map(function (todo) {
            return __h(
                'li',
                { className: theme.item, key: todo.id },
                __h(_TodoView2.default, { todo: todo })
            );
        })
    );
}

_CustomReflect2.default.defineMetadata('design:subtype', 'jsx', TodosView);

_CustomReflect2.default.defineMetadata('design:paramtypes', [{
    theme: _TodosTheme2.default
}], TodosView);

Такой подход дает движок для рендеринга, react там или что-то, что поддерживает интерфейс React.createElement, отделить от кода приложения.

zerkalica avatar Nov 18 '16 08:11 zerkalica