Add Hacker News clone? #167
Replies: 39 comments
-
|
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. 😄 |
Beta Was this translation helpful? Give feedback.
-
|
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 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 I asked previously about effects, and your answer was about The component doesn't know when it gets mounted, and I tried So the extra 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? |
Beta Was this translation helpful? Give feedback.
-
|
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 |
Beta Was this translation helpful? Give feedback.
-
|
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 a few days I created this lib https://github.com/luwes/disco just for having explicit connected and disconnected events for DOM elements.
import { observe } from '@luwes/disco'; // waiting on the `disco` npm name 😄
observe('.comments-item'); |
Beta Was this translation helpful? Give feedback.
-
|
seems to work well with |
Beta Was this translation helpful? Give feedback.
-
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. |
Beta Was this translation helpful? Give feedback.
-
tbh, I'm not very fond of this approach either - reading the code still doesn't make it obvious what 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:
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? 🙂 |
Beta Was this translation helpful? Give feedback.
-
|
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 |
Beta Was this translation helpful? Give feedback.
-
|
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. |
Beta Was this translation helpful? Give feedback.
-
|
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. |
Beta Was this translation helpful? Give feedback.
-
|
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. |
Beta Was this translation helpful? Give feedback.
-
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: There's nothing that lets you know the app root has been mounted. Can't we just add a simple hook for that? 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. 🙂 |
Beta Was this translation helpful? Give feedback.
-
|
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.
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: |
Beta Was this translation helpful? Give feedback.
-
|
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.
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 |
Beta Was this translation helpful? Give feedback.
-
|
Maybe I didn't explain that idea well. 😊 When I say "hook", I'm not talking about React-like hooks or something like that - I'm using the word in the most general sense, meaning basically "something that gets called when something happens". The framework has What the framework doesn't know, is when do these nodes get added to the actual document - because that's not something the framework does right now, the userland code does that manually. All I'm suggesting a So after function notifyConnectedChildren(parent) {
parent.querySelectorAll("*").forEach(node => {
node.dispatchEvent(new Event("connected"));
});
}I'm ignoring some details like not broadcasting this event more than once - and this is a totally naive approach that won't scale or perform well at all, but that would work, right? I don't think this adds anything substantial to the footprint. Usage would be similar to Disco, you just add an event-listener. Alternatively, maybe the framework provides an optional submodule with a helper of some sort? I tried something here, which is similar to what I was doing anyhow - but if it's part of the framework, that seems less like a hack, since that means the package promises that the behavior of this function will correlate with the implementation details of the framework. It doesn't do much, it mostly prescribes a way to solve the problem, and removes any worries about relying on implementation details. That approach probably only works for connecting and not for disconnecting though. |
Beta Was this translation helpful? Give feedback.
-
|
I see, yes. Hmm. What do you think about the React-style "transition group" approach? It's essentially diffing internally, which isn't a really attractive prospect in the context of Sinuous - I don't think we want to duplicate that logic. But maybe something similar could be achieved with a component of some sort, which internally uses sinuous/map and places every child in a wrapper component to manage the animation or something..? |
Beta Was this translation helpful? Give feedback.
-
I can only speak for myself but if I liked diffing and vdom I'd be using React 😏 Sinuous is nice because it doesn't do that, but I still see what you mean. I'll try and think about how I'd design a tabular data form for input state and rows/cols that fade out. Do you have a more specific example of what you're building? Because the solution might not be to patch the core framework, it might be best implemented as logic at the component level. |
Beta Was this translation helpful? Give feedback.
-
|
@heyheyhello a minimal example could be just a list of names - so each row would be just an input and a remove-button. If that works, it should work for rows with multiple/different types of inputs too, right? |
Beta Was this translation helpful? Give feedback.
-
|
(by the way, another thing that's nice about Sinuous is, if you do have to diff with sinuous/map, it's shallow, not deep like it is in React - you're just adding/removing items from the list, not drilling into all the children of each item. Man, this library makes so much sense!) |
Beta Was this translation helpful? Give feedback.
-
|
Right but that doesn't inheritly need any diffing. It's just a list so you can use sinuous/map to render the items, and if you wanted to delay removal of items just set the remove button event listener to something like: onClick={() => {
element.className += "fade500"
setTimeout(() => nameListObservable(nameListObservable().splice(...)), 500)
}}(I'm typing on a phone so gimme some slack) but you'd basically not involve map in the pre removal phase at all... Just invoke map by changing the observable after you've done all your pre remove stuff |
Beta Was this translation helpful? Give feedback.
-
|
Yes.. people are strangely adverse to using set timeout. Claiming it is hacky. It isn't limited to conversation earlier in this thread. It comes up every time. Something like transition group isn't too bad. It too can be shallow comparison. It's just a transforming buffer. It remembers its previous state and marks/collects nodes that are new/old and returns the hybrid state. This way in something like the example above only the new row would fade in not the whole thing. Or only the single row would fade out. If it were just limited to top level nodes it would probably be not too difficult to implement. The challenge is making sure multiple actions happening before the previous ended would properly work and not stomp on them. The easiest way is to slide itself between the reactive mapping(data to DOM) and the DOM updates(old DOM to new DOM). I've seen map functions offer per row transformations but that might complicate the core reconciler to handle the case. If one could simply handle the intermediate list holistically before insert only the places that needed to pay the cost would. Anyway this topic has been on my mind lately. There might be even better approaches. Svelte's animation is impressive and I suspect syntax aside something similar can be done in this area. |
Beta Was this translation helpful? Give feedback.
-
|
Not averse to timeouts at all. 😉 But delaying the removal itself can be problematic - you're delaying the update of the model, which means your model could be temporarily in an inconsistent state. Or you'd have to add "removing" properties to your items, which means you're putting UI details in your model and dealing with them explicitly. For example, say you're removing an item from a shopping cart. If I remove an time, I'd want the total to update immediately - not half a second later when the animation ends. You can accomplish this with a
I took a look at Svelte's animation demo and frankly I'm not that impressed. The animation itself looks good, but things are still popping/jumping when you add/remove something. The technique they applied looks solid though. There's an older library implementing this pattern, might be useful for reference. I wonder if it's possible to combine the FLIP technique with a wrapper or placeholder element and animate the width/height of that, so that surrounding items would correctly transition into place while the item's occupied space shrinks or expands... 🤔 |
Beta Was this translation helpful? Give feedback.
-
|
Closing here. Thanks for the great conversations! ❤️ |
Beta Was this translation helpful? Give feedback.
-
|
I thought of a simple idea to solve this, maybe. I would just add something to the API, like: function mount(node) {
if (node.ownerDocument.contains(node)) {
node.dispatchEvent("mount", new Event(mount));
}
return node;
}And then, the procedure for mounting something in the DOM would be e.g.: mount(document.appendChild(<App/>));The library internally would call Granted, this doesn't work for components that don't render any elements - but, typically, the reason you're interested in knowing when elements get mounted, is because you need to measure their size, or something layout-dependent, and this should work well enough? Not sure if an |
Beta Was this translation helpful? Give feedback.
-
|
@mindplay-dk This is what my sinuous-lifecycle package does, specifically tracking when children nodes join parents to form a larger tree without yet being mounted to the page, and then when it is mounted all children are notified. I knew Node#contains would be a simpler solution but was concerned about how much work that entails under the hood:
In both Chrome and Firefox there's the full walk of the ancestors of a node and the consideration of whether they're in the right document, shadow root, etc. I can't tell how expensive it is. It might be a lot faster than JS walking, but I opted to assume the worse when writing sinuous-trace, so I build a tree of components (and guards email me if you wanna chat), under the idea that you'll be able to skip a lot of nodes by hopping... Kinda like https://en.wikipedia.org/wiki/Saltatory_conduction haha ⚡ No idea if I'm right. There's a chance the C++ implementations are so fast that any WeakMap hopping in JS doesn't even compare. |
Beta Was this translation helpful? Give feedback.
-
|
@heyheyhello there's also |
Beta Was this translation helpful? Give feedback.
-
|
Might be. Chrome uses a flag it seems for isConnected. Its own bookmarking system for nodes. Looks even O(1) at first glance |
Beta Was this translation helpful? Give feedback.
-
|
Hm, events of course were designed for things like UI events - clicks, etc... so they bubble from their target towards the document root - which is the opposite of what I wanted, to broadcast an event to all of the child-nodes. That's not a feature of the DOM event system, it seems. Events go up, not down. (well, they go down during capture, but only along the parent path to the root.) I mean, you could DOM events don't seem like the way to go. Hmm, I really want to think of a simple solution to this. 🤔 How about collecting all newly created nodes in a global array? Whenever I say "global", I feel dirty, but... We could simply collect all the nodes pending that "mount" notification, and then flush that list and broadcast notifications after connecting a node to the document? Would that be... bad? |
Beta Was this translation helpful? Give feedback.
-
|
I implemented this in my toy sinuous clone - it seems to work well enough, and it's simpler than I thought it would be: const isConnected = 'isConnected' in Node.prototype
? node => node.isConnected // modern browsers
: node => node.ownerDocument.contains(node); // IE11 and older browsers
let onConnectListeners = [];
export function onConnect(f) {
onConnectListeners.push(f);
}
export function connect(node) {
if (isConnected(node)) {
onConnectListeners.forEach(f => f());
onConnectListeners = [];
}
return node;
}
Components register for notification with When you bootstrap your app, you have to call connect(document.getElementById("app").appendChild(<MyApp/>));Internally, I call Usage in my example looks like this: function Counter({ value }) {
const count = observable(value);
let div = (
<div>
<button onClick={() => count.set(count.get() - 1)}>-</button>
<input type="number" value={() => count.get()}/>
<button onClick={() => count.set(count.get() + 1)}>+</button>
</div>
);
onConnect(() => {
console.log("Hello from Counter! at y-position:", div.getBoundingClientRect().y);
});
return div;
}If you toggle the "show counters" checkbox, you can see this working for a reactive expression - and if you press the "add item" button, you can see it working for It's a pretty simple solution - what do you think? Am I missing something incredibly obvious here? 🙂 |
Beta Was this translation helpful? Give feedback.
-
|
Huh, it's even simpler than that - we don't even need to check if the node is connected. By the time we call the let onConnectListeners = [];
export function onConnect(f) {
onConnectListeners.push(f);
}
export function connect(node) {
onConnectListeners.forEach(f => f());
onConnectListeners = [];
return node;
}We don't even need to know which node we're dealing with - the Again, this is true in my toy sinuous clone at least, where creation and reactive updates are always distinct - at least so far, with reactive expressions and (I've tried several things by now to come up with a simple life-cycle hook for removals, and this seems much harder, since there's no "component instance" with this type of architecture.) |
Beta Was this translation helpful? Give feedback.
-
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? 🙂
Beta Was this translation helpful? Give feedback.
All reactions