The Wayback Machine - https://web.archive.org/web/20201213011157/https://github.com/ReactiveDB/core/pull/114
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

Feat: traces in QueryToken #114

Open
wants to merge 7 commits into
base: master
from
Open

Feat: traces in QueryToken #114

wants to merge 7 commits into from

Conversation

@WuHuiling
Copy link
Collaborator

@WuHuiling WuHuiling commented Apr 30, 2019

No description provided.

@WuHuiling WuHuiling requested a review from Saviio Apr 30, 2019
@WuHuiling WuHuiling self-assigned this Apr 30, 2019
@WuHuiling WuHuiling changed the title Feat/QueryToken traces Feat: traces in QueryToken Apr 30, 2019
@WuHuiling WuHuiling removed their assignment Apr 30, 2019
@WuHuiling WuHuiling requested a review from chuan6 Apr 30, 2019
@chuan6
Copy link
Contributor

@chuan6 chuan6 commented May 6, 2019

@WuHuiling @Saviio 👏

我这几天看一下,给出代码层面的反馈 👌

src/utils/diff.ts Outdated Show resolved Hide resolved
src/utils/diff.ts Outdated Show resolved Hide resolved
src/utils/diff.ts Outdated Show resolved Hide resolved
src/utils/diff.ts Outdated Show resolved Hide resolved
src/version.ts Outdated Show resolved Hide resolved
@WuHuiling WuHuiling force-pushed the feature/query-token-traces branch from a304e44 to 221be31 May 6, 2019
@coveralls
Copy link

@coveralls coveralls commented May 6, 2019

Coverage Status

Coverage increased (+0.2%) to 96.885% when pulling 11cb1cc on feature/query-token-traces into 5cbff58 on master.

@Saviio
Copy link
Contributor

@Saviio Saviio commented May 6, 2019

LGTM

src/utils/diff.ts Show resolved Hide resolved
@WuHuiling WuHuiling force-pushed the feature/query-token-traces branch from 221be31 to 0b45bed May 8, 2019
@chuan6 chuan6 force-pushed the feature/query-token-traces branch 2 times, most recently from 580ef67 to 3d1ec55 Jul 22, 2019
@chuan6
Copy link
Contributor

@chuan6 chuan6 commented Jul 22, 2019

@Saviio 我上面将 v0.10.4-alpha.11-diff 这个标签下的代码结合到这个 PR 的代码上来了。这么做是希望之后这里的代码修改就是我们实际使用的代码修改。(结合过程中,我确保了这些修改放到使用 rxjs5 的低版本 rdb 上也是同样可用的。)

这样之后发版时,将这里的 commits cherry-pick 到老版本 rdb 上就可以了。维护一份修改好追踪一些 :)

@chuan6 chuan6 force-pushed the feature/query-token-traces branch 3 times, most recently from ea5a3ed to 5f45087 Jul 22, 2019
const op: Op = isEqual ? { type: OpType.Reuse, index: prevIndex } : { type: OpType.New, index: k }

if (prevIndex === k && isEqual) {
reused++

This comment has been minimized.

@chuan6

chuan6 Jul 22, 2019
Contributor

@Saviio 我能把这个 reused 变量改名叫作 reuseValueAndIndex 吗?以与上边的 OpType.Reuse 区别。

@chuan6 chuan6 force-pushed the feature/query-token-traces branch from 5f45087 to 8156526 Jul 24, 2019
...并调整 QueryToken 里的 lastEmit 字段初始值,避免使用不易与实际推出
结果区分的 `[]`,使用 undefined;traces 接口在遇到 lastEmit 尚未设值的
情况时,返回意为全新结果集的 TraceResult。
@@ -174,7 +174,7 @@ export function diff<T>(oldList: T[], newList: T[], pk = '_id'): Ops {
}
}

const arrayIsSame = reused === curr.length && prev.length === curr.length
const arrayIsSame = reused === curr.length && prev.length === curr.length && reused !== 0

This comment has been minimized.

@chuan6

chuan6 Jul 25, 2019
Contributor

@Saviio 这个添加的 reused !== 0 条件是不是为了区分 QueryToken 里 lastEmit 初始值用的?

我在这个 commit 里把 lastEmit 初始值调整为 undefined,同时在 traces() 接口的实现里,多出一个分支,处理初次推出值的情况。

想请你帮忙确认一下,这里的 reused !== 0 在我这么调整后是否还需要?谢谢 :)

chuan6 added 2 commits Jul 25, 2019
This reverts commit 45de6bf.
...满足 Observable 订阅类似于函数调用,二次调用不应该出现与一次调用不
同的情况,否则行为难以预知。将 trace 的概念从 lastEmit 中区分开来,作
为 QueryToken 级的变量,而不受订阅影响。
Copy link
Collaborator

@DomonJi DomonJi left a comment

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.