Upgrade to React v16 lifecycle #1975

zhujinxuan · 2018-07-17T03:07:50Z

Is this adding or improving a feature or fixing a bug?

feature

What's the new behavior?

use memoize-one instead of state as recommended in reactJS https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html#what-about-memoization
value is handled by and only props.onChange, then no state.value is provided.
Remove flushChange in componentDidUpdate. After mount, all update will go to onEvent->change->onChange->props.onChange

Does this fix any issues or need any specific reviewers?

Fixes: #1426 #1427
Reviewers: @

wmertens · 2018-07-17T05:11:17Z

packages/slate-react/src/components/editor.js

+    // the updates, then warn the user that they may be doing something wrong.
+    if (this.tmp.resolves > 5 && this.tmp.resolves == this.tmp.updates) {
+      logger.warn(
+        'A Slate <Editor> is re-resolving `props.plugins` or `props.schema` on each update, which leads to poor performance. This is often due to passing in a new `schema` or `plugins` prop with each render by declaring them inline in your render function. Do not do this!'


how about making that === 5 so you only warn once?

This warning only appears in dev mode. We want the warn appears in each update, so the developer cannot miss it.

zhujinxuan · 2018-07-17T14:41:09Z

This PR is ready for review~

jtadmor · 2018-07-17T18:23:13Z

packages/slate-react/src/components/content.js

-      obj[handler] = this[handler]
-      return obj
-    }, {})
+    const handlers = this.getHandlers()


im not sure why the initial implementation is the way it is, but this is actually creating new behavior (new function was being created before on every render, now its not).

are we sure this doesn't subtly change anything?

Because

We do not need to re-evaluate a new object

We do not need to mixin a lot of onXXX functions

Therefore I use a getHandlers for this part of logic

jtadmor · 2018-07-17T18:25:33Z

packages/slate-react/src/components/editor.js

@@ -5,6 +5,7 @@ import SlateTypes from 'slate-prop-types'
 import Types from 'prop-types'
 import logger from 'slate-dev-logger'
 import { Schema, Stack } from 'slate'
+import memoize from 'memoize-one'


do we want to call this memoizeOne or something similar, where other parts of the codebase use other memoization strategies?

I think, for react component in slate-react, we only have this memoize currently. I may change the name when the memoize strategy becomes more complex.

I agree, I think calling it memoizeOne is a good idea here.

jtadmor · 2018-07-17T18:45:39Z

packages/slate-react/src/components/editor.js

+   */
+
+  getStackWithMemoization = memoize(plugins => {
+    this.tmp.resolves++


should we move this.tmp.resolves++ into this.resolvePlugins just for consistency?

Yes. Thank you. Fixed~

jtadmor · 2018-07-17T18:46:54Z

packages/slate-react/src/components/editor.js

+   */
+
+  processValueOnChange = memoize((value, stack) => {
+    if (this.tmp.change && value === this.tmp.change.value) return value


if we are going to memoize this, should we explicitly pass in this.tmp.change.value as an argument?

or are there other protections against this.tmp.change.value changing between calls to this method while value and stack stay the same?

This part is to ensure processValueOnChange(processValueOnChange(value)) === processValueOnChange(value)

The reason is that, in each update, we will have stack.run('onChange', change) passed to the parent container, then the parent container will pass stack.run('onChange', change) back as props.value. And I do not want to have the stack.run('onChange', change) run again when receiving props.

I do not see a neat way to solve the problem. The only thing I can do is to ensure all value are updates are passed via processValueOnChange

@jtadmor Hi, I have just removed this part of logic in recent commit, and put the value verification in didMount and onChange. I agree with you that we shall leave the memoized function pure.

Cheers,

jtadmor · 2018-07-17T18:49:35Z

packages/slate-react/src/components/editor.js

    const { onChange } = this.props
-    if (value == this.value) return
-    onChange(change)
+    if (value === this.value) return


isnt this.value just a getter that resolves to the same thing as line 269 right above? seems like this will cause it to abort on every run.

ianstormtaylor

Hey @zhujinxuan thanks for this. Can you explain what the goal of this is, or why it's important? Also what you mean by "removing flushChange"?

ianstormtaylor · 2018-07-17T19:58:36Z

packages/slate-react/src/components/leaf.js

      props.text != this.props.text ||
-      props.parent != this.props.parent


What are these changes for?

Because marks can be the same value with different reference. Therefore I think it is perhaps better with immutable equals

Aren't these changes unrelated to the rest of this pull request though?

ianstormtaylor · 2018-07-17T20:00:54Z

packages/slate-react/src/components/editor.js

+   * @return {Stack}
+   */
+
+  getStackWithMemoization = memoize(plugins => {


Can you name these resolveStack/resolveSchema to match the existing resolvePlugins?

Why not have these call resolvePlugins internally themselves instead of needing to take in plugins? (Might be a good reason for it, I'm just quickly reading.)

Because we need a argument to make the function working with right cache. Therefore we would like to pass the plugins. If we do not have plugins as argument, then it will always return the same value regardless of plugins change

ianstormtaylor · 2018-07-17T20:01:18Z

packages/slate-react/src/components/editor.js

    const { onChange } = this.props
-    if (value == this.value) return
-    onChange(change)
+    this.tmp.value = value


All our values are updated by this.props.onChange and stack.run('onChange'). During the mount, we pass the stack.run('onChange', this.props.value) back to the onChange, and the the parent container feed use the value after onChange.

But we do not want to have onChange called again on the value that we returned to the parent container and then passed back to me. Because calling onChange again and again on the same value will dangerously lead to infinite loop.

Therefore I use a tmp.value and tell the get value() not to run onChange if it is already processed by onChange(https://github.com/ianstormtaylor/slate/pull/1975/files#diff-0f1805ffc056f7c54e6a6f98ba0259a1R174)

zhujinxuan · 2018-07-18T00:11:09Z

Hi, @ianstormtaylor . The main goal of this PR is to remove the deprecated lifecycle componentWillReceiveProps

The React team is suggesting to use memoize-one or other memoize functions instead of componentWillReceiveProps (https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html#what-about-memoization) in usual cases. Then I find that state.value is un-necessary because we always get value from parent container and update value by callback from parent container. Then we use a memoize function to remove the state.value

Because state no longer handles value, and all value updates to passed by event handlers and onChange function, then we do not need to use componentDidUpdate to update value. Then flushChange and queueChange is no longer necessary.

ianstormtaylor · 2018-07-31T23:50:30Z

packages/slate-react/src/components/content.js

-      }
-    })
-  }
+  getHandlers = memoize(() => {


It seems like there's no reason to memoize this? Instead we could just build the this.handlers object in the constructor instead, and always refer to it. That would be less confusing I think. (Or maybe even class properties allow you to use this without needing the constructor even?)

ianstormtaylor · 2018-07-31T23:51:17Z

packages/slate-react/src/components/editor.js

@@ -5,6 +5,7 @@ import SlateTypes from 'slate-prop-types'
 import Types from 'prop-types'
 import logger from 'slate-dev-logger'
 import { Schema, Stack } from 'slate'
+import memoize from 'memoize-one'


I agree, I think calling it memoizeOne is a good idea here.

ianstormtaylor · 2018-07-31T23:52:06Z

packages/slate-react/src/components/editor.js

-        this.onEvent(handler, ...args)
-      }
-    })
+  tmp = {


This is a nice simplification!

ianstormtaylor · 2018-07-31T23:54:18Z

packages/slate-react/src/components/editor.js

-      this.props.onChange(change)
-    }
-  }
+  getHandlers = memoize(() =>


Same here for this, no reason to memoize it seems.

ianstormtaylor · 2018-07-31T23:56:05Z

packages/slate-react/src/components/editor.js

+  normalizeOnChangeValue = memoize((value, stack) => {
+    const change = value.change()
+    stack.run('onChange', change, this)
+    return change.value


This doesn't make sense to me, we're losing the change.operations here because they are not being exposed to the user. This doesn't work for collaborative editing.

Oh, I see. onChange must return a change with all operations in this cycle. I am working on that.

Oh, I see. I will use this memoize function only in get value; As for onChange function, we will run stack.run('onChange', change, this) instead of calling this function.

Fixed~ this memoize function will be only used in get value

ianstormtaylor · 2018-07-31T23:56:25Z

packages/slate-react/src/components/editor.js

-    if (value == this.value) return
-    onChange(change)
+    this.tmp.value = value
+    onChange(value.change())


This also doesn't make sense to return a new change object. It should return the change object with the actual operations on it.

ianstormtaylor · 2018-07-31T23:57:23Z

packages/slate-react/src/components/leaf.js

      props.text != this.props.text ||
-      props.parent != this.props.parent


Aren't these changes unrelated to the rest of this pull request though?

ianstormtaylor · 2018-08-01T00:00:26Z

packages/slate-react/src/plugins/after.js

-      obj[handler] = editor[handler]
-      return obj
-    }, {})
+    const handlers = editor.getHandlers()


I like how handlers have been simplified in other parts of this pull request. But I think here it feels strange. This is probably because it's a bit weird that we're rendering <Content> here, instead of passing it through as a child in the Editor.render. Could you change that? Then there will be no reaching for those handlers.

ianstormtaylor · 2018-08-01T00:01:30Z

packages/slate-react/src/components/editor.js

-    this.state.stack = stack
-
-    // Run `onChange` on the passed-in value because we need to ensure that it
-    // is normalized, and queue the resulting change.


Where has this logic gone? This is important to ensure that the editor never renders a value that is invalid according to the schema.

@ianstormtaylor This part of logic has gone to get value

get value use the resolved stack, and use the resolved stack to get value after onChange function.

Oh, I see. Do you mean if stack changes and value is unchanged, we shall still run onChange? This is fixed in 5188877

With the current logic on master, if the <Editor> is passed an invalid value, it will normalize it against the schema before rendering. And then after mounting it will ensure that the operations in that extra normalization pass are propagated to onChange so that the view layer is aware.

@ianstormtaylor I see. The normalize logic is put into get value() in https://github.com/zhujinxuan/slate/blob/ef7e626bc980f7680aec751a633e7699d7be4eee/packages/slate-react/src/components/editor.js#L173;
get value will get value from either last this.onChange function or associateStackAndValue (https://github.com/zhujinxuan/slate/blob/ef7e626bc980f7680aec751a633e7699d7be4eee/packages/slate-react/src/components/editor.js#L195). Both have normalized value by stack.run('onChange)

zhujinxuan · 2018-08-07T23:10:14Z

Hi, @ianstormtaylor . I think I am missing some of your points. Can I know what the current problem is with this PR?

ianstormtaylor · 2018-08-07T23:34:14Z

@zhujinxuan it's a bit hard to follow, but it looks better now. But I think it's still not compatible with the current behavior. Specifically this case:

Create an invalid value object, according to their schema.
Render an <Editor> with the invalid valid.
The editor should automatically normalize the value to make it valid.
The editor should only ever render with a valid value.
Once mounted, the editor should call its own onChange with the new value.
The change object should include the operations that normalization applied.

It looks like now this PR does everything but 6. which is required for collaborative editing implementations to be able to know what operations were performed. It looks like it's creating a new change object instead as onChange(value.change()) which means that the change.operations is empty.

codecov · 2018-08-08T00:21:01Z

Codecov Report

Merging #1975 into master will decrease coverage by 0.03%.
The diff coverage is 53.42%.

@@            Coverage Diff             @@
##           master    #1975      +/-   ##
==========================================
- Coverage   66.82%   66.78%   -0.04%     
==========================================
  Files          68       68              
  Lines        5624     5624              
==========================================
- Hits         3758     3756       -2     
- Misses       1866     1868       +2

Impacted Files	Coverage Δ
packages/slate-react/src/components/node.js	`60.86% <ø> (ø)`	⬆️
packages/slate-react/src/components/content.js	`23.23% <100%> (-4.13%)`	⬇️
packages/slate-react/src/plugins/after.js	`4.61% <100%> (-0.73%)`	⬇️
packages/slate-react/src/components/editor.js	`52.94% <50.72%> (+1.62%)`	⬆️
packages/slate/src/models/schema.js	`88.38% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55f0999...a67ae9d. Read the comment docs.

zhujinxuan · 2018-08-08T00:50:38Z

@ianstormtaylor How about this version? I queue changes with a Map and clear the Map after each update to prevent memory leak.

ianstormtaylor · 2018-08-08T01:11:21Z

I honestly don't understand what's going on there. What's the identity function for? Why are all the changes being cached in a Map at all? Why does that code path need to change in the first place?

This isn't something I'm likely to merge if it makes the logic (which already wasn't obvious) even more confusing than before.

zhujinxuan · 2018-08-08T03:39:21Z

@ianstormtaylor I see. I just find that using id function and Map is unnecessary. I make an update that works in this way:

in get value, triggered by a change of props.
1.1 If the value is same as the value from the last lifecycle, use the value; otherwise
1.2 queue the change when get value is triggered
In componentDidMount, componentDidUpdate and onEvent, call this.change to update change:
2.1 Get change from queued change.
2.2 If no operation happens since this.props.value, do not call props.onChange (when onEvent triggers value change, then the following componentDidUpdate shall not update the value again)
2.3 queue the value and stack. Therefore the get value do not re-evaluate stack.onChange on a value which is already run with onChange.

ianstormtaylor

Hey @zhujinxuan, nice this is looking better! Few more comments inline.

ianstormtaylor · 2018-08-08T23:03:02Z

packages/slate-react/src/components/editor.js

    }
+
+    this.tmp.updates++


Why does updates increment here?

This updates calculates the number of completed life cycles. It will be compared to tmp.resolves to decide whether to warn in componentDidUpdate that schema is re-created too many times.

It is ++ in DidMount because in the first mount, resolvePlugins will increase the tmp.resolves.

ianstormtaylor · 2018-08-08T23:08:43Z

packages/slate-react/src/components/editor.js

+   * @param {Change} change
+   */
+
+  queueChange(change) {


Can you remove this function and inline the logic instead since it's only used in one place now?

ianstormtaylor · 2018-08-08T23:13:52Z

packages/slate-react/src/components/editor.js

-   * @param {Change} change
-   */
-
-  onChange = change => {


We can't remove this editor.onChange(change) function because it's part of the public API that people are depending on. At least for now.

Fixed. onChnage is restored in 37502ee

ianstormtaylor · 2018-08-10T17:45:37Z

packages/slate-react/src/components/editor.js

+   * @return {Change}
+   */
+
+  associateStackAndValue = memoizeOne((value, stack) => {


Can you call this resolveValue to be consistent with the other places?

ianstormtaylor · 2018-08-10T17:45:54Z

packages/slate-react/src/components/editor.js

  }

  get value() {
-    return this.state.value
+    if (this.tmp.value === this.props.value && this.tmp.stack === this.stack) {


Can you add a comment here to explain what this is doing?

ianstormtaylor · 2018-08-10T17:52:15Z

packages/slate-react/src/components/editor.js

+    debug('onChange', { change })
+    const { value } = change
+    if (value == this.props.value && change.operations.size === 0) return
+    this.stack.run('onChange', change, this)


Thanks for re-adding this onChange method.

We've now got another issue with this logic though, which is that there are three different places that stack.run('onChange' is called from, which I don't think is good because it makes things hard to follow.

Right now, it seems like one of the reasons for the duplication is that in the componentDidMount we're doing this.change(change => change) to actually flush any existing changes. This feels like too much magic, it isn't clear. And if that line was refactored to be more clear and not depend on the same code path, I think it would allow us to remove the duplcation.

For example (pseudocode):

if (this.tmp.change) { this.flushChange() }

Ideally editor.change should be reduced to:

change = (...args) => { const { value } = this const change = value.change() change.call(...args) this.onChange(change) }

Otherwise it probably means we're doing something too confusing.

Hi, @ianstormtaylor . How about 3257e3e? The problem is that onChange(change) can take an arbitrary change that not restricted to the inner state of the editor. So we have to decide whether to run stack.run(onChange) in difference places. One for inner change state this.tmp.change in this.change, the other for arbitrary change in this.onChange

ianstormtaylor · 2018-08-10T17:52:53Z

packages/slate-react/src/components/editor.js

+   * @param {Change} change
+  */
+
+  updateChange = change => {


Similarly, I think this updateChange method should be only called from a single place, and then it wouldn't need to be its own method.

zhujinxuan · 2018-08-16T21:35:48Z

Update: this PR shall be able to close #1426 and #1427

ianstormtaylor · 2018-08-16T21:59:22Z

Thanks @zhujinxuan! I've added a single commit there which refactors things a bit, and cleans up some of the comments and logging. It will hopefully make the code slightly easier to follow. (Allow it will still be confusing because memoization is inherently confusing I think.)

Thanks for working on this!

* Trying to use memoization and upgrade to react v16 * Fix error * Fix error * Fix handlers error * Add annotation * Remove EventHandlers * No state * Remove un-necessary polyfill * Remove un-necessary polyfill * Remove un-necessary handlers settings * Early Return * Fix Early Return * Fix onChange * Do not run onChange stack twice on same change * Update annotation * Better sense of resolve++ * Cache value in onChange and didMount * Remove un-necessary rechack * Renaming * Remove change in leaf.js * Handlers as this.handlers * do not re-initialize change in onChange * Re-run onChange stack only when change happens * Update value when stack changes * Rename to memoize-one * queue changes * Unify interface * Fix bug * Add document * Remove id * Do not use map * Fix bug * Fix eslint * Fix update when props.value changes * Add annotation * Fix stack * Inline queueChange * Restore onChange * restore onChange * Refactor change and onChange * Use onChange as the single interface for update * Do not flushChange if inside event * Give a warning about synchronous editor.change call * Change isInChange in editor.change * refactor resolution and tmp logic, cleanup code

zhujinxuan mentioned this pull request Jul 17, 2018

[On Development] Prepare for React-v17 #1867

Closed

wmertens reviewed Jul 17, 2018

View reviewed changes

isubasti mentioned this pull request Jul 17, 2018

Reduce number of component updates #1938

Closed

zhujinxuan changed the title ~~[Working in Progress] Upgrade to React v16 lifecycle~~ Upgrade to React v16 lifecycle Jul 17, 2018

zhujinxuan changed the title ~~Upgrade to React v16 lifecycle~~ [Working in progress] Upgrade to React v16 lifecycle Jul 17, 2018

zhujinxuan changed the title ~~[Working in progress] Upgrade to React v16 lifecycle~~ Upgrade to React v16 lifecycle Jul 17, 2018

jtadmor reviewed Jul 17, 2018

View reviewed changes

ianstormtaylor reviewed Jul 17, 2018

View reviewed changes

zhujinxuan force-pushed the slate-react-v16 branch from 8c37241 to eeed3d5 Compare July 29, 2018 22:46

ianstormtaylor reviewed Aug 1, 2018

View reviewed changes

zhujinxuan force-pushed the slate-react-v16 branch from 144ded1 to e5e670d Compare August 1, 2018 00:55

zhujinxuan force-pushed the slate-react-v16 branch from ef7e626 to 5a9c9f4 Compare August 8, 2018 00:17

zhujinxuan mentioned this pull request Aug 8, 2018

onChange fires multiple times #2055

Closed

ianstormtaylor reviewed Aug 8, 2018

View reviewed changes

zhujinxuan force-pushed the slate-react-v16 branch from c8faf84 to 3a2bc68 Compare August 10, 2018 02:27

ianstormtaylor reviewed Aug 10, 2018

View reviewed changes

zhujinxuan force-pushed the slate-react-v16 branch from 8173c45 to 3257e3e Compare August 12, 2018 19:41

Trying to use memoization and upgrade to react v16

48d23f0

zhujinxuan added 17 commits August 15, 2018 16:09

Rename to memoize-one

8a74fe8

queue changes

9e86c43

Unify interface

d6994a3

Fix bug

b4899c5

Add document

a80fa70

Remove id

ca9fbc2

Do not use map

4a05458

Fix bug

7b437bd

Fix eslint

905e18c

Fix update when props.value changes

b15ca57

Add annotation

68807a1

Fix stack

60d5e49

Inline queueChange

7ae6f23

Restore onChange

cd18b1f

restore onChange

9f97651

Refactor change and onChange

4f05b74

Use onChange as the single interface for update

6ce866e

zhujinxuan force-pushed the slate-react-v16 branch 2 times, most recently from 8719c34 to 6ce866e Compare August 15, 2018 20:15

zhujinxuan added 3 commits August 15, 2018 16:20

Do not flushChange if inside event

20b95bc

Give a warning about synchronous editor.change call

c4d72c2

Change isInChange in editor.change

4d44396

refactor resolution and tmp logic, cleanup code

a67ae9d

ianstormtaylor added the debt label Aug 16, 2018

ianstormtaylor merged commit 877dea1 into ianstormtaylor:master Aug 16, 2018

Nantris mentioned this pull request Aug 26, 2018

Slate-React - Unable to focus editor programmatically #2097

Closed

zhujinxuan mentioned this pull request Sep 19, 2018

Fixing Editor validation/normalize after schema prop change. #1427

Closed

		props.text != this.props.text \|\|
		props.parent != this.props.parent

Upgrade to React v16 lifecycle #1975

Upgrade to React v16 lifecycle #1975

Conversation

zhujinxuan commented Jul 17, 2018 • edited Loading

Is this adding or improving a feature or fixing a bug?

What's the new behavior?

Does this fix any issues or need any specific reviewers?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhujinxuan commented Jul 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhujinxuan Jul 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhujinxuan Jul 17, 2018 • edited Loading

Choose a reason for hiding this comment

jtadmor Jul 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianstormtaylor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhujinxuan Jul 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhujinxuan commented Jul 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhujinxuan Aug 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhujinxuan Aug 1, 2018 • edited Loading

Choose a reason for hiding this comment

ianstormtaylor Aug 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhujinxuan commented Aug 7, 2018

ianstormtaylor commented Aug 7, 2018 • edited Loading

codecov bot commented Aug 8, 2018 • edited Loading

Codecov Report

zhujinxuan commented Aug 8, 2018 • edited Loading

ianstormtaylor commented Aug 8, 2018

zhujinxuan commented Aug 8, 2018 • edited Loading

ianstormtaylor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhujinxuan Aug 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhujinxuan commented Aug 16, 2018

ianstormtaylor commented Aug 16, 2018

zhujinxuan commented Jul 17, 2018 •

edited

Loading

zhujinxuan Jul 18, 2018 •

edited

Loading

zhujinxuan Jul 17, 2018 •

edited

Loading

jtadmor Jul 17, 2018 •

edited

Loading

zhujinxuan Jul 17, 2018 •

edited

Loading

zhujinxuan Aug 1, 2018 •

edited

Loading

zhujinxuan Aug 1, 2018 •

edited

Loading

ianstormtaylor Aug 1, 2018 •

edited

Loading

ianstormtaylor commented Aug 7, 2018 •

edited

Loading

codecov bot commented Aug 8, 2018 •

edited

Loading

zhujinxuan commented Aug 8, 2018 •

edited

Loading

zhujinxuan commented Aug 8, 2018 •

edited

Loading

zhujinxuan Aug 10, 2018 •

edited

Loading