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

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

Open
zerkalica opened this issue Nov 12, 2016 · 9 comments
Open

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

zerkalica opened this issue Nov 12, 2016 · 9 comments

Comments

@zerkalica
Copy link

zerkalica commented Nov 12, 2016

Предлагаю ввести понятие метаданных атома, которые тоже являются атомом, но просачиваются сквозь 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
Copy link
Author

zerkalica commented Nov 13, 2016

В 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')
        }
    })

@Riim
Copy link
Owner

Riim commented Nov 14, 2016

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

@zerkalica
Copy link
Author

В случае 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())

Riim added a commit that referenced this issue Nov 17, 2016
@Riim
Copy link
Owner

Riim commented Nov 17, 2016

Добавил Cell#getStatus.

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

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

@zerkalica
Copy link
Author

правильнее было назвать 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
Copy link
Author

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

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

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

Riim added a commit that referenced this issue Nov 18, 2016
@Riim
Copy link
Owner

Riim commented Nov 18, 2016

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

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

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

@Riim
Copy link
Owner

Riim commented Nov 18, 2016

Добавил Cell#reap.

@zerkalica
Copy link
Author

zerkalica commented Nov 18, 2016

Зачем вообще 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, отделить от кода приложения.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants