Add Hacker News clone?

[mindplay-dk]

So I think I'm done with my Hacker News clone for now:

https://codesandbox.io/s/sinuous-hacker-news-dqtf7

I would have liked to add some progressive loading - it's pretty aggressive on the HTTP requests now, since every comment in a thread requires recursive HTTP requests for the individual items... probably a bit bad on items with too many comments.

Let me know what you think? 🙂

[mindplay-dk]

Uhm, in case you already looked - no idea what happened, but CS had not been saving my changes to one of the files, apparently it was just pretending. 😐

Fortunately, there was almost nothing in that file, it was easy to recreate.

Should be working now. 😄

[mindplay-dk]

There is one thing I'm not happy with, and I'm not sure if there's a better way to do this.

If you look at reveal.js, this is how I create the animation when CommentItem instances get recursively loaded and rendered in comments.js.

If I was working with plain DOM, what I'd use is something like this:

function appendTo(parent, child) {
  parent.appendChild(child);
  
  const height = child.offsetHeight;
  
  child.style.height = "0";
  child.style.transition = "0.5s";
  child.style.overflow = "hidden";
  
  requestAnimationFrame(() => setTimeout(() => child.style.height = `${height}px`));
}

That last line waits for the next animation frame, then waits for CSS to initialize the start of the transition before setting the height - I picked that up from this answer.

Unlike that function, the reveal function in this demo needs an extra requestAnimationFrame before measuring the offsetHeight - because you can't measure the height of an element until it's parented. It doesn't have to be rendered (and it isn't, since I'm immediately changing it's height to zero after measuring it) but it does have to be parented for the DOM to calculate the actual height based on the layout parent.

I asked previously about effects, and your answer was about cleanup, which for some reason I couldn't get to work here.

The component doesn't know when it gets mounted, and I tried cleanup, but it didn't seem to work for this case - offsetHeight was zero, so cleanup seems to happen before the component has been mounted. (?)

So the extra requestAnimationFrame is a work-around because, unlike appendTo, I'm not in charge of parenting the new element, and it's just sort of a "fingers crossed" approach, where I'm hoping Sinuous will have parented the element by the next animation frame so I can measure the size.

But that code is non-obvious and I'd prefer something more explicit that actually explains when/how it's doing this - it feels really brittle, and seems to rely on an implementation detail in Sinuous.

Is there a better (safer, more explicit) way to approach this?

[luwes]

waaw this is really cool! code wise it looks great to me.

one request for each comment is indeed quite taxing, if there is a comments collection endpoint that would be better.

not sure if it's intentional but since the stories endpoints are cached the generated DOM creation of the stories per type could also benefit from being cached. right now every top menu link click will re-render the whole list.

checking your last comment now

[luwes]

yes, cleanup is run when the computation re-runs. it runs on removal but also just when the computation is re-run. it can't work as a mount hook and there's not a super easy way to do it unfortunately.

related issue #73
worth checking @ryansolid's comments and solution too solidjs/solid#36 (comment)

a few days I created this lib https://github.com/luwes/disco just for having explicit connected and disconnected events for DOM elements.
at the moment I think that's the best way to go.

observe() takes string selectors too so something like the below would add a connected event to the comment items when it attaches to the DOM.

import { observe } from '@luwes/disco'; // waiting on the `disco` npm name 😄 

observe('.comments-item');

[luwes]

seems to work well with connected events https://codesandbox.io/s/sinuous-hacker-news-yx36w

[ryansolid]

So the extra requestAnimationFrame is a work-around because, unlike appendTo, I'm not in charge of parenting the new element, and it's just sort of a "fingers crossed" approach, where I'm hoping Sinuous will have parented the element by the next animation frame so I can measure the size.

I get the reaction, I've just done precisely this.. well setTimeout 0, so many times in the last 10 years of using KnockoutJS I stopped even thinking about it as a hack. I'm old school though, we did this with jQuery before that too. Because cleanups occur you can ensure you can cancel the timeout in the weird chance it is removed before you get the chance. The only reason I've come across to use mutation observers is if you managing DOM elements independently of the render lifecycle. The scenario in that issue was holding on to the Tabs that were disconnected when not selected and re-inserting them. In that case you care about actual connectivity. But for DOM measurements etc.. these libraries render synchronously. So even by the next microtask you are probably good.

PS. Awesome demo.

[mindplay-dk]

seems to work well with connected events

tbh, I'm not very fond of this approach either - reading the code still doesn't make it obvious what observe('.comments-item') does or why onconnected works.

I understand how it works, but I'd be concerned about using something like this at scale or in a team - like when you come back to an old jQuery project and have no idea where listeners get attached or why dispatching a click-event somewhere does anything at all.

Any chance for some kind of native events in the framework?

I'm thinking back to an older version of this package, which had these incredibly useful element-level life-cycle events, which enabled things like animations on creation/removal - for example:

      <div class="notifications__item notifications__item--hidden"
      key={ item.key }
      oncreate={ el => setTimeout(() => el.classList.remove('notifications__item--hidden')) }
      onremove={ (el, done) => {
        el.classList.add('notifications__item--hidden');
        setTimeout(done, 1000);
      } }>

onremove here was a personal favorite - being able to defer the removal of an element was incredibly useful for things like deleting items from lists.

Not saying element-level life-cycle events are necessarily the right approach - this is just an example of something that was a good fit in a different framework.

I mean, surely the framework knows when it's adding/removing elements? Without having to ask the DOM to observe and notify it of changes that it itself requested? 🙂

Disco looks pretty cool and I could think of some good uses - I've actually often wished for something like this, and I've seen some pretty horrible solutions in the wild where something similar was achieved by polling the DOM and repeating queries. 😬

But in the context of Sinuous, going around the framework back to the DOM, to see what the framework is doing... it's... not elegant? I'd prefer the framework itself provided an answer.

Tall order? 🙂

[luwes]

that's the thing, the framework doesn't know when a created element is attached to the DOM. 😃 it's because it creates inside out, being based on the hyperscript like calls.

h('div', 'text child', h('div', 'div child'));

you can see here, the right most div is created before its parent and the parent might not be attached to the DOM yet.

that's one advantage of VDOM. just thought of re-dom, how do they do it? hmm

[ryansolid]

I think it is just what they do. That is the library(ReDOM) essentially. Their whole system is managing this connected state. It is always a bit of a challenge for single pass libraries not based on DOM lifecycles (basically every non-VDOM library that doesn't use Web Components) but especially challenging for reactive libraries that have no centralized render cycle.

It is possible to do here but it requires a whole another layer of book-keeping. If you collected the nodes that would be connected for the first time on each operations, and would be removed you could iterate over the list at the end of each update. You basically need to add ReDOM on top of everything we are already doing and then add the overhead to scheduling of reactive updates.

I think the only solution is based on the type of work queue a job and maybe attach information on the DOM nodes themselves. I looked into this and ultimately decided it wasn't worth it and if someone really cared a mutation observer approach made sense since it is so easy to add. Like disco or even closer to disconnected where they masquerade as DOM events. Even adding that sort of functionality to the library would be simpler than actually trying to track additions/removals like ReDOM.

I know this was already linked but like it's so simple. https://codesandbox.io/s/dom-lifecycle-u0gbd. Is library agnostic and does the trick and the consumer is no wiser. Because ultimately even though it's a whole other 2nd pass any approach we took would be too.

[luwes]

good you posted your example again, I read it wrong before. thought it had some performance drawbacks but it's probably the most performant way to do it. checking if the dis-connected methods exist on the node.

[ryansolid]

Yeah I mean it does have the overhead of walking that portion of the tree each time something is added or removed. But I mean it comes with the territory if you want this behavior. It's a single mutation observer so it's not bad for performance. I'm not going to add it to a benchmark for sure but it's reasonable.

[mindplay-dk]

that's the thing, the framework doesn't know when a created element is attached to the DOM. 😃 it's because it creates inside out, being based on the hyperscript like calls.

Hmm, yeah, the library is not responsible for adding the root node of an app to the document - but it is responsible for adding all the nodes to their parent nodes, right?

It's also responsible for inserting nodes into the existing structure during updates.

So the only thing you don't have a hook for, is when the structure you've created gets added to the document - you'd have hooks for every other insert/update/delete operation.

So it seems maybe the problem is how apps start up:

document.body.appendChild(App());

There's nothing that lets you know the app root has been mounted.

Can't we just add a simple hook for that?

mount(App(), document.body);

And I guess maybe that hook just broadcasts a "connected" message to all the children, similar to what you're doing with disco.

Most other frameworks have an initialization hook, so I don't think a requirement like that would be too surprising to anyone?

@ryansolid to your last comment, I'm not really too concerned about the performance - though MutationObserver does need a much slower polyfill in IE, but it's more the principle of using an external facility to solve an internal architectural problem that bothers me.

Even if it solves the problem, the framework shouldn't need a facility to tell the framework what the framework itself just did. Should it? It's quite possible I'm missing something here. 🙂

[ryansolid]

Hmm.. I suppose if you are going to handle it from a DOM node perspective there are options. I was mostly considering this from a Component perspective since that's what VDOM libraries do and where you get a concrete framework like benefit.. Like a hook. It is more consistent than adding arbitrary handlers on a per element basis. Nothing else in the library does that. And that's a much tougher problem since Components are ephemeral. They are just function calls.

It should be possible to check the connected state of the parent node (isConnected, polyfillable by Mutation Observer in IE11) at any insert point and then basically run the same code as the Mutation Observer to check down the tree. There may be performance cost/complexity especially on removal as shortcuts couldn't be taken. Although it sort of the same thing as the mutation observer solution and if built that way that behavior wouldn't be able to opt-in/opt-out of that behavior as easy. I suppose some sort of global config. But to an end user(who didn't need to worry about polyfills) both solutions would be indistinguishable. You include the package or you don't (via import, config etc).

Basically from the libraries perspective DOM updates are side effects. It'a a bit weird to think of it that way. But there is no data structure it's managing that is DOM specific, no VDOM, no linked DOM tree templates, no Component hierarchy. Just independently executable reactive nodes. We just happen to put some DOM insert and cleanup code in them for rendering. No cell is really aware of the whole system. All context is sort of injected orthogonally. Which means the skies the limit on the powerful things we can model with these simple primitives, but by default they don't know what they are doing. I've modelled Suspense and other VDOM mechanisms without touching the rendering at all. It's all independent. Someone in the Community could have done it and released their own package without even changing the library. Which is very different than say Preact's migration to Preact X.

So there are a number of options from simplest to most complicated.

1. Consider this a data propagation timing issue due to a side effect and set a timeout.
2. Add a mutation observer to the root and run DOM node specific methods on addition/removal.
3. Add connected checks to renderer to inserts/removals that do some DOM node specific post processing.
4. Add connected checks to renderer to inserts/removals + mark nodes for Components with component id (including handling fragments), so that during these operations we can use a context lookup to run mounted/unmounted hooks for each Component rather than handling it on a per DOM node basis.

There are probably a few others. Definitely interesting areas to try. Personally in the past decade of using KnockoutJS I never really moved past 1.

EDIT:
@luwes I apologize I kind of got carried away as usual. I think there are some options here. I'm not going to push my opinion too hard here as Sinuous is a completely independent project than my perspective of the overall approach.

[luwes]

all good @ryansolid, it's very useful information.

@mindplay-dk I'll look into this further, maybe redom has a simple solution for it however to add it to the Sinuous core will probably add too many kB's. I'd like to keep the hello world example below 1.5kB.

Even if it solves the problem, the framework shouldn't need a facility to tell the framework what the framework itself just did. Should it?

one one side I agree but on the other I also think the DOM api's got the element appended and then being able to solve the connected and disconnected hooks/events with other native api's is not a bad way to do it.

the observe() function from disco does look somewhat out of context, maybe this can be improved with more explicit api's.