Git Product home page Git Product logo

Comments (6)

atirip avatar atirip commented on May 24, 2024

So far, inserting this:

let l = futureNodes.length;
while (l--) if (futureNodes[l] instanceof Wire) futureNodes.splice(l, 1, ...futureNodes[l].childNodes);

in the beginning of the domdiff() seems to fix it for me.

from hyperhtml.

WebReflection avatar WebReflection commented on May 24, 2024

domdiff knows nothing about wires, but I am struggling to understand what are you trying to do here.

hyperHTML owns its content, so that you should never mess up with it manually.

However, if you try this instead, I see no errors:

const {bind, wire} = hyperHTML;
var props = {}

var a = wire()`<p>A</p><p>A2</p>`;
var b = wire()`<p>B</p><p>B2</p>`;
var c = wire()`<p>C</p><p>C2</p>`;
var d = wire()`<p>D</p><p>D2</p>`;

function render() {
  console.log('rendering');
  var x;
  
  if (Math.random() < 0.5) {
    x = a
    a = c
    c = x
  }

  var r = wire()`
    <div>
      ${[a, b]}
    </div>
  `;
  var r2 = wire()`
    <div>
      ${[c, d]}
    </div>
  `;
  return [r, r2];
}

setInterval(
  bound => bound`<section>${render()}</section>`,
  1000,
  bind(document.body)
);

The explanation is quite simple: if you hard wire nodes and you manually move hard wired nodes around, the engine wouldn't know where these nodes are: 1 wire, 1 or more nodes. Move manually the node somewhere else, the wire doesn't know anymore what to handle so that dropping the hard wired content fixes this.

Since in 2+ years I've never had to do anything like you did here, and this library has been in production for more than 1 year and a half already, and since you are manually interfering with hard wired nodes, I suggest you take a look at lighterhtml instead, because it deals with non-keyed results in a way that hyperHTML might never do.

Your example would look like the following one:

const {render, html} = lighterhtml;

let a = html`<p>A</p><p>A2</p>`;
let b = html`<p>B</p><p>B2</p>`;
let c = html`<p>C</p><p>C2</p>`;
let d = html`<p>D</p><p>D2</p>`;

function update() {
  console.log('rendering');
  let x;
  
  if (Math.random() < 0.5) {
    x = a
    a = c
    c = x
  }

  const r = html`
    <div>
      ${a}
      ${b}
    </div>
  `;
  const r2 = html`
    <div>
      ${c}
      ${d}
    </div>
  `;
  return [r, r2];
}

setInterval(
  () => render(document.body, html`<section>${update()}</section>`),
  1000
);

and you can test it live on CodePen.

from hyperhtml.

atirip avatar atirip commented on May 24, 2024

This is just and example that exposes specific case where domdiff crashes. It just bite me lately, while I have been using hyperHTML for a long time. Just I have DOM that occasionally changes a lot. Also I need to use props and wireId's, because I have a lots of form elements and I need to avoid layout trashing at any cost.

  1. when futureNodes are Wire's instead of dom nodes, domdiff returns Wire's
  2. these Wire's are stored into childNodes
  3. if and only if some next call needs to rearrange nodes, then domdiff uses these childNodes, but that fails.
  4. it fails, because Wire has 'dead', parentless nodes inside.

I do not see why you accuse me manually moving nodes, I do not move nothing manually, even in the example there. And I can fix that with the short code above myself. How can you explain that, my 'dewire' fix magically heals my 'manual' moves.

Anyway, you are too abrasive and unpleasant to work with. I can just fork.

from hyperhtml.

WebReflection avatar WebReflection commented on May 24, 2024

Once again, domdiff has nothing to do with this: it accepts a get(node) function provided by hyperHTML.

domdiff also doesn't crash, as it's heavily tested and 100% code covered with all possible weird cases you want, including this one.

I do not move nothing manually

You are swapping hard wired internal content via random, but it's not about accusing anyone, it's just the pattern you used that is not a common one. You create wires somewhere else and you use these in any hole arbitrarily, which is not a scenario well handled by hyperHTML, because it works best with keyed nodes.

When you move nodes arbitrarily, you could have same node in two different places, as example, and that's an issue.

On top of that, you are using fragments (more than a node within a template literal and within the same non-existent root), so fragments are like that, you move a fragment around, you'll leave its nodes whenever they were, and this messes up with the tagger.

However, this is the issue in a nutshell:

  • first div has a and b, then it has c and b, but a was moved somewhere else ... the moment hyperHTML remove a because its diff says it shouldn't be there, a could be removed from another parentNode that is not the one owned by the hard wired container
  • when that happens, you have issues, because the other container had c, now it doesn't anymore, so that diffing/moving any node around c at that point is not possible, as c is somewhere else

With lighterhtml handling non keyed cases, like this one, you don't have DOM trashing, just updates when things are different per node, and you'll never have the issue a node that supposed to be in a specific parent, is now somewhere else.

That being said, if you believe there is a domdiff bug in here, then this is the wrong repository for that, but I still suggest you move to lighterhtml which has handling of non keyed cases in core, while hyperHTML doesn't.

I might re-open this just to have a better look, but it's clear to me already why things aren't working as expected, and I think that's by design: you should not move wired content around, you should rather create unique wires per container. That's a keyed case that should work without issues.

from hyperhtml.

WebReflection avatar WebReflection commented on May 24, 2024

Anyway, you are too abrasive and unpleasant to work with. I can just fork.

I actually replied before reading this ... and now I feel bad because I've wasted time trying to explain things to you so ... I am closing this, as I have no interest in being insulted for free.

from hyperhtml.

WebReflection avatar WebReflection commented on May 24, 2024

Explaining, investigating, suggest and create alternative live examples => you are too abrasive and unpleasant to work with

Congratulations for your awesome Open Source contribution, remember that once you fork this project, which is used in production by more than a company, you should also follow its LICENSE terms.

I will block you next time you are incapable of having a respectful conversation.

from hyperhtml.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.