#113 - Implement experimental web worker support #114

treshugart · 2016-12-01T04:24:14Z

Implements #40, #113.

Implements #113

treshugart · 2016-12-01T04:24:29Z

gulpfile.js

@@ -1 +0,0 @@
-require('skatejs-build');


Boyscout: totes unused.

The build doesn't work now, is it because of this?

treshugart · 2016-12-01T04:27:07Z

test/unit.js

+    sd.diff({
+      destination,
+      source,
+      done (instructions) {


This is the tentatively proposed API: provide a done callback and if workers are available it uses them. Still need to patch render / merge to handle this.

treshugart · 2016-12-01T04:27:29Z

src/vdom/element.js

@@ -66,10 +66,12 @@ function translateFromReact (item) {
  return item;
 }

+let count = 0;


Definitely better ways, but this gets the initial spike working.

treshugart · 2016-12-01T04:28:01Z

src/util/real-node-map.js

+// import WeakMap from './weak-map';
+// export default new WeakMap();
+const map = [];
+export default {


Can't use weakmaps if we're using scalar values as keys.

treshugart · 2016-12-01T04:28:25Z

src/index.js


 export default {
  diff,
  merge,
  patch,
  render,
  types,
-  vdom,
-  version


Boyscout: removed this as it was way out of sync.

treshugart · 2016-12-01T12:17:34Z

Running sk-build should work.

…

On Thu., 1 Dec. 2016, 23:08 Matthew Phillips, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In gulpfile.js <#114>: > @@ -1 +0,0 @@ -require('skatejs-build'); The build doesn't work now, is it because of this? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#114>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIVbBMb0VazNQKA1jEc6--k1uD4ISdFks5rDrjagaJpZM4LA_VS> .

treshugart · 2016-12-01T12:17:49Z

On mobile. Will try in an hour or so when on lappy.

…

On Thu., 1 Dec. 2016, 23:17 Trey Shugart, ***@***.***> wrote: Running sk-build should work. On Thu., 1 Dec. 2016, 23:08 Matthew Phillips, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In gulpfile.js <#114>: > @@ -1 +0,0 @@ -require('skatejs-build'); The build doesn't work now, is it because of this? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#114>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIVbBMb0VazNQKA1jEc6--k1uD4ISdFks5rDrjagaJpZM4LA_VS> .

matthewp · 2016-12-01T12:28:24Z

Hm, I don't have a node_modules/.bin/sk-build, is that something I need to install globally?

treshugart · 2016-12-01T12:30:35Z

Darn. Npm install should do that. Can't check atm but can in 45mins ish.

…

On Thu., 1 Dec. 2016, 23:28 Matthew Phillips, ***@***.***> wrote: Hm, I don't have a node_modules/.bin/sk-build, is that something I need to install globally? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#114 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIVbF797H7_bHs9QQf4-VXfKADpfI7tks5rDr1ogaJpZM4LA_VS> .

matthewp · 2016-12-01T12:51:02Z

Are you using both rollup and webpack? Or maybe transitioning from one to another?

matthewp · 2016-12-01T12:56:12Z

About the API, I think it's a good API for a lot of use cases, but in my case I'm already running inside of a web worker, so I don't need to launch another worker. I'll be diffing 2 plain objects in the worker, sending the patch instructions back to the window and applying the patches there.

To keep my file-size down I'll probably use rollup, does the use of the webpack plugin here break rollup usage (even if you don't wind up using that functionality)? I could maybe just pull in dom-diff/src/diff-main.js directly since that's what I need on the worker since, then pull in dom-diff/src/patch.js on the window side.

treshugart · 2016-12-01T13:01:54Z

It should produce an artefact for the worker that you can load from dist for diffing.

…

On Thu., 1 Dec. 2016, 23:56 Matthew Phillips, ***@***.***> wrote: About the API, I think it's a good API for a lot of use cases, but in my case I'm already running inside of a web worker, so I don't need to launch another worker. I'll be diffing 2 plain objects in the worker, sending the patch instructions back to the window and applying the patches there. To keep my file-size down I'll probably use rollup, does the use of the webpack plugin here break rollup usage (even if you don't wind up using that functionality)? I could maybe just pull in dom-diff/src/diff-main.js directly since that's what I need on the worker since, then pull in dom-diff/src/patch.js on the window side. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#114 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIVbGOUkhBRGpr_QwuNVbWjLIT8wpkvks5rDsPsgaJpZM4LA_VS> .

treshugart · 2016-12-01T13:04:04Z

Either way, yeah, you should be able to load the diff main in a worker, that's what diff-worker does. You just need the deps that the worker-loader gives you.

…

On Fri., 2 Dec. 2016, 00:01 Trey Shugart, ***@***.***> wrote: It should produce an artefact for the worker that you can load from dist for diffing. On Thu., 1 Dec. 2016, 23:56 Matthew Phillips, ***@***.***> wrote: About the API, I think it's a good API for a lot of use cases, but in my case I'm already running inside of a web worker, so I don't need to launch another worker. I'll be diffing 2 plain objects in the worker, sending the patch instructions back to the window and applying the patches there. To keep my file-size down I'll probably use rollup, does the use of the webpack plugin here break rollup usage (even if you don't wind up using that functionality)? I could maybe just pull in dom-diff/src/diff-main.js directly since that's what I need on the worker since, then pull in dom-diff/src/patch.js on the window side. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#114 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIVbGOUkhBRGpr_QwuNVbWjLIT8wpkvks5rDsPsgaJpZM4LA_VS> .

treshugart · 2016-12-01T21:38:55Z

Fixed the rollup build issue. Now generating a dist/index.js with webpack and will add a min version soon.

matthewp · 2016-12-01T21:43:30Z

So, I don't know if you answered this, but since you are using jsnext config, a rollup user might do:

import { diff } from 'skatejs-dom-diff';

Will this work or blow up, aren't you using a webpack config in src/diff.js or am I just confused?

treshugart · 2016-12-01T21:46:55Z

You're right. I'll remove that. Travelling between Syd and Mel right now which is why my responses have been patchy :) I'll have a look a bit later.

…

On Fri., 2 Dec. 2016, 08:43 Matthew Phillips, ***@***.***> wrote: So, I don't know if you answered this, but since you are using jsnext config, a rollup user might do: import { diff } from 'skatejs-dom-diff'; Will this work or blow up, aren't you using a webpack config in src/diff.js or am I just confused? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#114 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIVbOQruULht3WdLjv8a1Ybsa8SHdFMks5rDz-CgaJpZM4LA_VS> .

matthewp · 2016-12-01T22:48:33Z

ids don't seem to work, I guess because it assumes your nodes are created with sd.vdom.element, right? So that means it only works with patching 2 vdom nodes. You can't patch a real node against a vdom node or vice versa.

Also, if one Node is created in the worker and another in the window their ids aren't going to match up. I'm pretty sure this is why React has ids that look like 0.1.14.2.3, it's really a path to find the Node. That's the only way to find the same Node from another tree.

I wrote a library for this purpose about a year ago: https://github.com/canjs/node-route/blob/master/src/dom-id.js The inline docs do a decent job of explaining what is going on. Of particular interest are nodeRoute.getID(node) which gives you that id like "0.1.14.2.3". And other is nodeRoute.getNode(id, root) which will find the Node for a given id. Notice that it uses firstChild and nextSibling which your vdom doesn't have, so its not something you could drop in and use unfortunately. But could be a guide if you wanted to use it.

This could wind up being a bigger change that you were probably anticipating, I imagine, so I would understand if you don't want to pursue it further.

treshugart · 2016-12-02T01:01:26Z

ids don't seem to work, I guess because it assumes your nodes are created with sd.vdom.element, right? So that means it only works with patching 2 vdom nodes. You can't patch a real node against a vdom node or vice versa.

This is intentional as there's no way to transfer a DOM node from the real DOM to a web worker. A way around this would be to extract information from each node and give it an id at that point in time. Unfortunately this would involve building a virtual tree from the entire real tree on the main thread. Basically the inverse of the vdom/dom function. This might be worth doing if performance isn't the main concern, which there's definitely use cases for, but if that's the case, then why use web workers?

What's the use case for needing to diff a real tree in a worker?

matthewp · 2016-12-02T11:54:07Z

You need to construct a virtual tree from the real tree once, when the page loads, if you ever intend to patch the real tree. I want to build the virtual tree on page load, send it to a worker, and from that point on only patch that virtual tree against other virtual trees, sending the patch instructions back to the window and applying the patches on the real tree.

…ent and predictable.

…e unnecessary ones.

…onversion. Remove old APIs. BREAKING CHANGE: major refactor

treshugart · 2016-12-07T09:49:57Z

@matthewp

Massive update. I've added both toDom() and toVdom() to convert between real and virtual trees. You should be able to do something like:

import { diff, toVdom } from 'skatejs-dom-diff';

const vTree = toVdom(document.getElementById('app'));
diff(vTree, myNewTree, {
  done (instructions) {
    patch(instructions);
  }
});

That said, the render() function now supports doing all that; so it will take the root node you provide it and convert it to a virtual tree before diffing and patching. It also supports the done function for diffing in a worker:

import { render } from 'skatejs-dom-diff';

const renderer = render(root => (
  <div>I was rendered in {root.id}!</div>
));

renderer(document.getElementById('app'));

Understand if you've moved on, but I've quite enjoyed just tinkering on it. Fun project.

matthewp · 2016-12-07T18:28:45Z

Yeah no doubt, I think this is a real differentiator for the project, as I've explored all of them just about and most do not break out diffing and patching in this way.

I'm happy with idom but definitely am not married to it. I have plans for creating benchmarking for vdom through web workers approaches. In the meantime I still have a branch of my project using dom diff so I'll try out these changes and let you know.

treshugart added 2 commits December 1, 2016 12:14

refactor: Remove out of sync version file. Remove unnecessary gulp file.

aaa321b

feat: Add experimental web worker support.

3e36a5e

Implements #113

treshugart commented Dec 1, 2016

View reviewed changes

treshugart added 5 commits December 1, 2016 15:30

refactor: Fix bad equality check.

5e88c1e

refactor: let -> const

d46282f

refactor: Update merge() and render() to accept a done callback.

ff5dba0

refactor(test): Refactor assertion for async renders.

d80c6aa

docs: Update docs for web workers.

166fe0b

build: Fix some build issues. Still need a index.min.js.

b95cc2e

treshugart added 3 commits December 5, 2016 19:25

refactor: Refactor tests and start major refactor to make api consist…

22a4f5d

…ent and predictable.

refactor: Simplify code.

8ce2f30

refactor: Massive major refactor. Added many features and removed som…

e776b96

…e unnecessary ones.

treshugart added 3 commits December 7, 2016 20:37

feat: Update docs. Add web worker support. Add to / from dom / vdom c…

27a1a9b

…onversion. Remove old APIs. BREAKING CHANGE: major refactor

refactor: Try and fix IE.

dc04d36

refactor: Don't use object.assign because IE

2296875

treshugart merged commit 766e774 into master Dec 10, 2016

treshugart deleted the 113 branch December 10, 2016 02:41

		@@ -1 +0,0 @@
		require('skatejs-build');

#113 - Implement experimental web worker support #114

#113 - Implement experimental web worker support #114

Uh oh!

Conversation

treshugart commented Dec 1, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

treshugart Dec 1, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthewp Dec 1, 2016

Choose a reason for hiding this comment

Uh oh!

treshugart Dec 1, 2016

Choose a reason for hiding this comment

Uh oh!

treshugart Dec 1, 2016

Choose a reason for hiding this comment

Uh oh!

treshugart Dec 1, 2016

Choose a reason for hiding this comment

Uh oh!

treshugart Dec 1, 2016

Choose a reason for hiding this comment

Uh oh!

treshugart commented Dec 1, 2016 via email

Uh oh!

treshugart commented Dec 1, 2016 via email

Uh oh!

matthewp commented Dec 1, 2016

Uh oh!

treshugart commented Dec 1, 2016 via email

Uh oh!

matthewp commented Dec 1, 2016

Uh oh!

matthewp commented Dec 1, 2016

Uh oh!

treshugart commented Dec 1, 2016 via email

Uh oh!

treshugart commented Dec 1, 2016 via email

Uh oh!

treshugart commented Dec 1, 2016

Uh oh!

matthewp commented Dec 1, 2016

Uh oh!

treshugart commented Dec 1, 2016 via email

Uh oh!

matthewp commented Dec 1, 2016

Uh oh!

treshugart commented Dec 2, 2016

Uh oh!

matthewp commented Dec 2, 2016

Uh oh!

treshugart commented Dec 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matthewp commented Dec 7, 2016

Uh oh!

Uh oh!

treshugart commented Dec 1, 2016 •

edited

Loading

treshugart Dec 1, 2016 •

edited

Loading

treshugart commented Dec 7, 2016 •

edited

Loading