-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
async rendering of children: should be properly coordinated #68
Comments
The name for this type of bug that I’ve heard is “tearing,” and I think you might find some helpful info if you search for that term in the other framework issue trackers. Here’s a sandbox with your code. I guess, my response would be that this is the correct behavior for those two components. Typically, you should lift your async/stateful logic into as high an ancestor if you can, same as React, and if you need to synchronize two components you should do so in a shared parent with async function* Parent() {
let i = 1;
this.addEventListener("click", () => {
i++;
this.refresh();
});
for await (const _ of this) {
await Promise.all([wait(1000), wait(200)]);
yield (
<div>
<A i={i} />
<B i={i} />
</div>
);
}
} I guess if I knew more about the actual async calls you’re trying to synchronize I could offer additional suggestions, but that’s what I suggest for your current example. |
First, feel free to close this issue whenever you want. I did not come here with the goal to troll or to annoy you. I understand your answer. But I think that it is a consequence of a "weak" asynchronous model: you push the coordination logic to the application layer. Your answer give the responsability to the end developer of making it work. This means that:
Note that all standard frameworks currently have the same issue. They require the developer to write/organize the code around the framework's limitation. This goes against the "component" model: my component leaks into the parent. And it is really hard when you have a modular system where the parent does not know its children component beforehand (plugin/extensible system). If you are interested by the discussion, let me give you a more concrete example, which is the exact same scenario: we have 3 components,
The And moving the asynchronous logic out of these two sub components is not what we want either. The problem goes deep, and if we have a complex tree of components, then we need to do some complex plumbing at each component to coordinate it with its children... (been there, done that) |
@ged-odoo Don’t think you’re trolling, and you’re definitely not annoying me (yet 😈). I agree that it would be nice for parents to coordinate child components to appear together. The most advanced work on this stuff currently is the work by the React team with SuspenseLists. SuspenseList provides a |
Well, I would say that my own framework is more advanced than react then :)... For your information, we actually tackled this very problem last year. Our initial design was using internally promises, probably similar to what Crank does internally. But it could not work in many more advanced cases. Situations where:
That last scenario in particular was close to impossible, because we cannot really properly replace promises inside a Our solution was to introduce a scheduler (about 100 loc), and an internal data structure (called |
Would something like this just work? Components are just functions after all? async function* Parent() {
let i = 1;
this.addEventListener("click", () => {
i++;
this.refresh();
});
for await (const _ of this) {
const A = await A({i});
const B = await B({i});
// Or Promise.all(...)
yield (
<div>
<A />
<B />
</div>
);
}
} If the components If this isn't possible your library could just expose smart and dumb components and businesses logic state accessors not unlike the async function* Parent() {
let i = 1;
this.addEventListener("click", () => {
i++;
this.refresh();
});
for await (const _ of this) {
const aState = await withA({i});
const bState = await withB({i});
// Or Promise.all(...)
yield (
<div>
<A {...aState}/>
<B {...bState}/>
</div>
);
}
} |
@nikordaris I’m personally against calling functions meant to be Crank components directly. It’s just too complicated and error-prone for the caller because you have to handle the four component return types ( |
That makes sense. I'm curious if there is a way to expose some utilities so we can manually render the components in the order we like and then stitch the results into the yielded jsx. Basically like the example I provided but having crank render it. Something like |
It’s going to be cumbersome but you could probably already get this done with the API crank exposes. async function *MyComponent({children}) {
this.schedule(() => this.refresh());
const div = yield <div>;
for ({children} of this) {
await renderer.render(children, div);
yield <Copy />;
}
} You can call the If you have more questions on this, it might be better to put it in a different issue just to keep this current issue clean. |
It should possible to create higher-order-components for this exact purpose: // Async component factory that returns a stateless sync component
async function A({i}) {
await wait(1000);
return function (this: Context) {
return <div>A:{i}</div>;
}
}
async function B({i}) {
await wait(200);
return function (this: Context) {
return <div>B: {i}</div>;
}
}
async function *Parent() {
let i = 1;
this.addEventListener('click', () => {
i++;
this.refresh();
});
for await (const _ of this) {
const A = await A({i});
const B = await B({i});
yield <div><A/><B/></div>;
}
} Didn't have a chance to check if this works, though. |
Dear Author of Crank,
as requested in a reddit thread (reddit ), here is a small scenario that I think is not handled properly by crank:
We have here a parent with two sub components A and B. These sub components render themselves at a different speed. Whenever I click on the parent, some information is propagated to the full sub tree.
What happens is that we see that B is updated first, and then A. I think that it is wrong. Both should be updated at the same moment. Otherwise, the UI is not in a consistent state.
The text was updated successfully, but these errors were encountered: