[WIP] attempted test of reconciler

[mindplay-dk]

This PR is for discussion only - not ready to merge.

Async testing (as required for testing the reconciler) with DOM in jest is new to me - I'm stuck.

My strategy with testRender() is to make two calls to render() - since the actual rendering is deferred, I'm relying on the update queue to indirectly indicate that the first render has completed, at which point the innerHTML of the target element should be populated.

But half the time, I get this error:

    console.error node_modules/jsdom/lib/jsdom/virtual-console.js:29
      Error: Uncaught [TypeError: Cannot read property 'base' of undefined]
          at reportException (/mnt/c/workspace/fre/node_modules/jsdom/lib/jsdom/living/helpers/runtime-script-errors.js:66:24)
          at Timeout.callback [as _onTimeout] (/mnt/c/workspace/fre/node_modules/jsdom/lib/jsdom/browser/Window.js:680:7)
          at ontimeout (timers.js:466:11)
          at tryOnTimeout (timers.js:304:5)
          at Timer.listOnTimeout (timers.js:267:5) TypeError: Cannot read property 'base' of undefined
          at base (/mnt/c/workspace/fre/src/reconciler.js:171:39)
          at commit (/mnt/c/workspace/fre/src/reconciler.js:161:5)
          at Array.forEach (<anonymous>)
          at forEach (/mnt/c/workspace/fre/src/reconciler.js:160:15)
          at commitWork (/mnt/c/workspace/fre/src/reconciler.js:42:11)
          at workLoop (/mnt/c/workspace/fre/src/reconciler.js:38:7)
          at Timeout.callback [as _onTimeout] (/mnt/c/workspace/fre/node_modules/jsdom/lib/jsdom/browser/Window.js:678:19)
          at ontimeout (timers.js:466:11)
          at tryOnTimeout (timers.js:304:5)
          at Timer.listOnTimeout (timers.js:267:5)

  ● render HTML elements

    TypeError: Cannot read property 'base' of undefined

      169 |   fiber.parent.patches = fiber.patches = []
      170 |   let parent = fiber.mum || fiber.parent.base
    > 171 |   let dom = fiber.base || fiber.child.base
          |                                       ^
      172 |   switch (fiber.patchTag) {
      173 |     case UPDATE:
      174 |       updateElement(dom, fiber.alternate.props, fiber.props)

      at base (src/reconciler.js:171:39)
      at commit (src/reconciler.js:161:5)
          at Array.forEach (<anonymous>)
      at forEach (src/reconciler.js:160:15)
      at commitWork (src/reconciler.js:42:11)
      at workLoop (src/reconciler.js:38:7)
      at Timeout.callback [as _onTimeout] (node_modules/jsdom/lib/jsdom/browser/Window.js:678:19)

And the other half of the time, I get a timeout, as the second render() never seems to complete at all.

@132yse any ideas?

(I also couldn't get async and await to work at all - the documentation doesn't help, and there's an issue with some solutions that didn't work for me either - so for now, I'm manually using .then() and calling done().)

[yisar]

I saw that the test of DOM in HA was written a testVdomToHtml function to test vdom results.
Is there any inspiration here?

[mindplay-dk]

HyperApp has no scheduler - DOM updates are synchronous, so it doesn’t really help with that.

I will look to see how React and Preact do this...

[yisar]

I need to explain:
I think we can test DOM results synchronously for two reasons:

1. Fre hasn't been scheduled yet. Now it has a time slice in it, only for reconciliation. The update rendering of DOM is synchronous yet.
2. As far as I know, react also has no test case for asynchronous rendering.

[yisar]

I haven't implemented priority scheduling yet 😭 . This may be done in v2. It will be a challenge.

[yisar]

I found that yesterday's PR about h function still had an impact on reconcilation, so I refactor it and now it's well.

[mindplay-dk]

The render() function calls scheduleWork, which calls defer(workLoop), so the rendering is deferred, it's not synchronous - I don't think it's synchronous in React either, I've seen them using the callback argument to render() in some tests at least.

Anyhow, I know what's wrong with this approach now: the functions in the reconciler rely on several global variables, so you can't make two consecutive calls to render() at all right now - because both will start their own deferred workLoop, and various functions will try to write to the same variables.

So, as of right now, there's no way to render() more than one thing at a time.

Major changes to reconciler are probably needed to make that possible? - but I think, for now, we need the render() callback argument, to at least make it possible to add tests.

[yisar]

I mean, instead of testing asynchronous rendering, we only test the final DOM result.
Now it's hard for me to think of ways to test workloop.
there may be a big change in the future. I want to implement priority scheduler, but then it will be more difficult to test.

How desperate We are! 😭

[mindplay-dk]

I mean, instead of testing asynchronous rendering, we only test the final DOM result.

How?

Rendering does not happen immediately - it goes into the workLoop first, which gets deffered.

To test the final DOM result, I need to know when rendering is done.

[yisar]

I have a idea to turn on synchronous rendering. Such as :

options.sync? workloop(): defer(workloop)

I'll refactor this part tomorrow to support synchronous rendering.
What do you think?

[mindplay-dk]

I don't think that's going to work? Under certain conditions (timing) the workLoop may schedule itself via defer(workLoop) - there are many calls to defer(workLoop), so I think it would be better to add the render() callback?

Also, testing the deferred workloop is actually important - if we avoid testing it, we won't be testing how it really works in a browser; we'd never get full test-coverage, so you can't be sure if you've broken something.

[mindplay-dk]

(Sorry about the mess, by the way - I have problems with my git installation, it seems it has replaced all lf with crlf and I can't get it to go back... I will probably scrap the whole PR and reopen it tomorrow.)

[yisar]

If there is a better way, we can try it.

If synchronous rendering is turned on, at least we can test other logic

In fact, I am not satisfied with Fre's time slicing right now. I want to implement priority scheduling, maybe work at the same time?

[yisar]

I have problems with my git installation

Haha, that's OK. We can continue our discussion.

[mindplay-dk]

In fact, I am not satisfied with Fre's time slicing right now. I want to implement priority scheduling, maybe work at the same time?

That would be amazing :-)

Didact is a good source of reference - there is proof that it does what it says it does, and an article that tries to explain it. (Frankly, I've tried, and it's over my head.)

Actually, I've tried to create a similar demo for Fre - I thought fibers were what made this work? I thought it was standard in React, actually. I can't seem to get it to work - it's got that CSS animation "glitch" either way... Maybe I'm doing something wrong?

[yisar]

Poor English limits my expression 😭 .
First of all, thank you for the use cases, which are greatful for the next work.

We see that the use case is not running smoothly, because fre now limits the threshold of 16 ms and wastes time if the task is less than 16 ms.

It's always slower than synchronous rendering, but it doesn't break the UI renderding.

Didact uses requestIdleCallback, tasks are low priority, but it can make full use of idle time

According to my understanding of pure functional views, I don't want to use this API. I want to reimplement the browser's stack as react does.

[mindplay-dk]

Closing this - there's a better PR coming in a moment!