Update fails with scrambled DOM result

[mindplay-dk]

I created the following small test example:

https://codesandbox.io/s/determined-butterfly-uxuic

At the top of the index.jsx file, you can swap the two import statements between fre and preact to see how this was supposed to work.

With preact:

With fre:

As you can see, the app breaks down as soon as you try to add another counter.

(Is key actually supported? Without support for key-based diffing, it's very hard to make any meaningful use of stateful functional components...)

[yisar]

@mindplay-dk I'm sorry, now fre's key is not valid for components, and only ordinary element's key is currently supported.

I've got a new idea here recently, and it's likely to come true soon.

Or do you have any suggestions?

[mindplay-dk]

Well, missing support for key probably should be tracked in a separate issue.

The main issue with this example is the DOM getting scrambled when main App component updates - that happens even if you remove the key attribute from the example.

[yisar]

Halo, I've fixed this case now, and now It can get the correct rendering, but the diff result is still not the smallest.
I've been busy in class recently. Please give me more time.

import { h, render, useState } from 'fre'

function App () {
  const [arr, setArr] = useState(1)
  return (
    <div>
      {new Array(arr).fill().map(i => (
        <A val={i} key={i}/>
      ))}
      <button onClick={() => setArr(arr+1)}>+</button>
    </div>
  )
}

function A (props) {
  const [count, setCount] = useState(0)
  return (
    <div>
      {count}
      <button onClick={() => setCount(count + 1)}>+</button>
    </div>
  )
}

render(<App />, document.body)

Or Could we study diff under fiber together?

[mindplay-dk]

I tried updating my example to 1.5.5, but I can't get the package to load at all now.

For example, if I do this:

import * as fre from "fre";
console.log(fre);

All I see in the console is { default: {} }.

It seems nothing is exported. The module build is maybe broken somehow?

[yisar]

@mindplay-dk I found some bugs in my diff scheme again. I will write a new diff scheme during the holidays. Thank you for your use case.

[yisar]

Ah ah! I found a JSX black magic to help solve this problem.
Please wait for my code pushing this weekend～

[yisar]

Rewrite the diff schema, fix this case, upgrade to the latest version.

[mindplay-dk]

The module works again - but now the elements aren't getting ordered correctly:

Also, the disabled-attribute on the "Remove" button doesn't seem to update anymore.

You can fork/test my example for yourself here:

https://codesandbox.io/s/determined-butterfly-uxuic

(You should probably think about adding at least a basic test-suite - complex things like these have a tendency to break very easily, even with small changes. It can be very difficult to manually test for every possible thing that could break.)

[yisar]

@mindplay-dk Agree. I have to admit that diff of the fiber list is really difficult. The main obstacle is the traversal of linked list.

I need more time to study it carefully. No other libraries have similar logic expect React, that makes it difficult for me to emulate.

Please wait for my reply, thanks!

[mindplay-dk]

You're brave for even attempting this 💪😁

[yisar]

@mindplay-dk Hey ～ I'm on vacation now. I've refactor a new version. It looks like it can solve the problem now.

This is not necessarily true, but the current way should be correct!

[mindplay-dk]

Works better now, components get created/destroyed and maintain state correctly. 👍

There's one problem remaining in my example though: the disabled property on the "Remove" button isn't being updated.

Looks like two things are missing here:

1. You're mapping props to setAttribute() calls, which only works for HTML attributes, which is not the same as object properties - for example, dom.disabled is a Boolean property, whereas dom.setAttribute("disabled", value) expects a string value; HTML attributes are always strings.

2. You only set attributes - you need to also be able to remove them.

You can see examples of both here. (Note that this implementation tracks whether a target element is an SVG node or not - something you may need to do later, if you want to support SVG. For reference, see also Preact's implementation here.)

[yisar]

Ah……you're right. It's my oversight. I'm going to fix it and support svg, too.

[yisar]

I fixed this and supported SVG…… but the size now reached 2kb！So sad 😢

[mindplay-dk]

2 KB for something like this is extremely good - focus on ensuring the project's integrity (tests!) and worry about optimizing for size later, when it's safe to do so. Refactoring for size at this stage, without tests, is extremely risky.

Do you have a preference for a test-framework? I see you have jest in there, so I might try to help out and get some basic tests going. 🙂

[yisar]

I really like testing framework T_T. but I'm not very good at it, but I think it's necessary.
Thank you very much !

[mindplay-dk]

I've confirmed the fix in 1.6.0 👍

I'm closing this issue, thanks! (Let's continue the discussion about tests elsewhere.)