solidjs / solid

A declarative, efficient, and flexible JavaScript library for building user interfaces.
https://solidjs.com
MIT License
32.05k stars 914 forks source link

Is it possible to have promise components? #83

Closed bluenote10 closed 4 years ago

bluenote10 commented 4 years ago

I'm trying to make sense on how asynchronous components would work in Solid. In particular I'm wondering if it is possible to use promises as components, enabling async/await usage like that:

import { render, Suspense } from "solid-js/dom";

async function PromiseComponent() {
  console.log("promise start");
  let data = await new Promise(resolve => setTimeout(resolve, 2000));
  console.log("promise fulfilled");
  return <div>Hello World</div>;
}

const App = () => {
  /* prettier-ignore */
  return (
    <div>
      <div>
        <div>Attempt 1</div>
        <PromiseComponent/>
      </div>
      <div>
        <div>Attempt 2</div>
        <Suspense>
          <PromiseComponent/>
        </Suspense>
      </div>
    </div>
  );
};

render(App, document.getElementById("app"));

That compiles, but doesn't behave like I would have expected. The documentation on Suspense doesn't really make it clear how it is supposed to work.

Looking at the examples on Suspense, I noticed that the AsyncChild actually isn't really async, i.e. it is not returning a Promise. Instead the examples make execution pseudo-synchronous by using loadResource. I would have liked to avoid that (for reasons like (1) await seems syntactically nicer / more standard, (2) I would have liked to avoid any wrapping overhead, (3) the wrapping destroys type information, (4) working with promises directly would maybe simplify composition). This makes me wonder if async function can actually be used as a component?

trusktr commented 4 years ago

You could write it something like the following (haven't tested it):

import { render, Suspense } from "solid-js/dom";

async function PromiseComponent() {
  console.log("promise start");
  let data = await new Promise(resolve => setTimeout(resolve, 2000));
  console.log("promise fulfilled");
  return <div>Hello World</div>;
}

const App = async () => {
  const [promisedDiv1, promisedDiv2] = await Promise.all([PromiseComponent(), PromiseComponent()])

  /* prettier-ignore */
  return (
    <div>
      <div>
        <div>Attempt 1</div>
        {promisedDiv1}
      </div>
      <div>
        <div>Attempt 2</div>
        <Suspense>
          {promisedDiv2}
        </Suspense>
      </div>
    </div>
  );
};

!async function main() {
  const app = document.getElementById("app")
  app.appendChild(await App())
}()

I haven't used Suspense before, so I left that there, but I'm guessing you don't need that, so attempt 2 is just like attempt 1:

        <div>Attempt 2</div>
        {promisedDiv2}
trusktr commented 4 years ago

The reason that should work (assuming you try it and fix any error I might have), is that after you await the promises, you get a reference to a DOM node, and SOlid allows you to compose DOM nodes by inserting them with {someDomNode}.

For example, you can do this:

const p = <p>{reactiveVariable}</p>
const d = <div>{p}</div>

where reactiveVariable is a reactive variable that gets updated over time, and that'll work.

trusktr commented 4 years ago

If you want to avoid splitting up your markup into pieces, you could also get references inline. For example:

let p
const d = (
  <div>
    {p = <p>{reactiveVariable}</p>}
  </div>
)
trusktr commented 4 years ago

Oh, last thing. You might be tempted to write something like

    <div>
      <div>
        <div>Attempt 1</div>
        {await PromiseComponent()}
      </div>
      <div>
        <div>Attempt 2</div>
        {await PromiseComponent()}
      </div>
    </div>

but at the moment I don't think that will work because Solid wraps the expressions in functions, which are not async functions, so that's why I awaited them outside of the markup.

ryansolid commented 4 years ago

I have experimented with a number of things here including pausing and resuming observable contexts. The tricky part is I can't inject myself inbetween the first stage of promise resolution and the continuation. Like if you view your Component as:

new Promise(resolve => setTimeout(resolve, 2000))
  .then(() => <div>Hello World</div>)

I can't get between the promise you create and the then. Computation tracking is context based so I need to be able to wrap the second execution. Like if something in the JSX was reactive. I tried to see if I had other means of continuation like throwing but it's awkward as unlike React we don't just restart the computation that started this whole thing. We have dependency tracking to consider. It can lead to wasted work as the boundaries aren't component boundaries, and I don't keep any actual local state, it would just hit the promise again rinse and repeat). Even if I were to track different execution states the work we do in computations can be heavy. This isn't fed into a diff engine. There is some JSX in there you are creating DOM nodes.

So in any case converting a Promise to a Synchronous node with Asyncronous resolution is by far the simplest. That way I can split the promise and the result. Essentially doing this:

const signal = wrapPromise(() => new Promise(resolve => setTimeout(resolve, 2000)))
const result = createMemo(() => signal() && <div>Hello World</div>)
insert(result)

Things can resolve asynchronously as long as the graph is created as part of its parent contexts execution (can be connected back to a root). It doesn't necessarily have to be in initial synchronous execution but we need to be able to resume in a wrapped state. That context is not only important for things like dynamic dependency tracking but to be able to use things like Solid's Suspense.

Now you trivial example where there is nothing dynamic after the promise resolution we could write a simple wrapper similar to Solid.lazy to handle that case and you'd be able to write an async Component like that. In theory I could even support that in insert. But I think it causes unexpected behavior because it exists outside of the runtime context. Because there is no opportunity to wrap I couldn't even create a different root if I wanted to unless the end user manually did so which doesn't make things any nicer for experience.

All that being said there might be some really tricky way to solve this I'm not seeing. I need to be able to wrap the the then for starters. I can't think currently anyway to do that without adding more syntax.

bluenote10 commented 4 years ago

@trusktr That is also something I have tried, but I couldn't really make it work, because the async functions are running without a root. For instance, this slight modification will should the "cleanups created without a root" warning:

import { render, Suspense, For } from "solid-js/dom";

async function PromiseComponent() {
  console.log("promise start");
  let data = await new Promise(resolve => setTimeout(resolve, 2000));
  console.log("promise fulfilled");
  return <div>Hello World</div>;
}

const App = async () => {
  const [promisedDiv1, promisedDiv2] = await Promise.all([PromiseComponent(), PromiseComponent()])

  /* prettier-ignore */
  return (
    <div>
      <div>
        <div>Attempt 1</div>
        {promisedDiv1}
      </div>
      <div>
        <div>Attempt 2</div>
        <Suspense>
          {promisedDiv2}
        </Suspense>
      </div>
      <For each={[1, 2, 3]}>{ i =>
        <div>{i}</div>
      }</For>
    </div>
  );
};

!async function main() {
  const el = document.getElementById("app");
  let app = await App()
  render(() => app, el);
}()

The problem I couldn't solve is basically that I would have to run render(() => await App(), el) to run with a root, but render itself doesn't accept async functions.

I still have to wrap my head around Ryan's comment though. This "simple wrapper similar to Solid.lazy to be able to write an async component" is basically what I was trying to implement, but couldn't figure out how. Or actually I thought that's the purpose of Suspense.

ryansolid commented 4 years ago

Yeah sorry guys because this leads into the hypothetical of things I was looking at recently I dumped too much detail. Generally speaking right now the answer is no in any meaningful way. Any async Component would be unable to have dynamic views.

I did debate briefly keeping the await syntax with a wrapper on the promise. That way I'd control the then and could continue the context although I'm unclear of the proper cleanup pattern. Like having loadResource return a promise you can await but not only is it more complex I didn't see the gains. The async await pattern lends to making flows more imperative than declarative. I could probably go this way if it made sense but I'd need to make the library promise aware(handle synchronous wrapping of promises when inserted) and change how suspense works. NVM (See Edit).

So looking at your concerns. 2 we are stuck with regardless of the variation at which point 1 is sort of mute since it's just extra syntax if you have to wrap. Unsurprisingly TypeScript doesn't like it. Not sure on that one. As for point 4 I mean loadResource also takes reactive function as an input so it can dynamically run on prop changes etc. You can create whatever promise pattern inside.

EDIT: Thinking about this more I remember why I landed here ultimately. Turning the renderer wholly async doesn't make sense with this model/approach. Think about the execution patterns. How would we handle promise rejection or retries. Or like a component that changes what if fetches based on props. You'd need to re execute the whole Component. There is no granularity control. Youd want to wrap this stuff in an effect or memo anyway and that is what loadResource does.

ryansolid commented 4 years ago

All that being said is there confusion about the suspense feature? The wrapping of Promises is actually what React does too. What Suspense does is provide a mount point at which we can suspend downstream DOM attachment until all async code is executed. So the nodes aren't mounted (or are unmounted) during this resolution. The key advantage of this approach unlike Show control flow is descendants are created allowing them to also trigger Suspense inverting control to the child instead of the parent. This allows for many variations of loading patterns as execution is never stopped hierarchically unless explicitly done.

The final piece I added was Control Flow awareness through transforms. While not important in the simple case this let's the maxDuration value hold the current value while evaluating the new path and only switching when Suspense is lifted. This allows for seamless navigation without flashes of white or loading spinners when content loads fast enough as you will continue to see the old content for the first half a second let's say if the content doesn't load immediately. Only after that time it shows loading state if still hasn't loaded. This has the uncanny effect of making the app feel way quicker even though technically it's less responsive. I demoed it for several people A/B testing style with random delays without saying what I was doing and when asked the difference they all said the Suspense with maxDuration version was preferable because it loaded faster most of the time. The actual load times were no different.

bluenote10 commented 4 years ago

All that being said is there confusion about the suspense feature?

I guess not. I have never touched Suspense in React, so I had no particular expectations. All I know is that in modern JS async/await makes writing async code very convenient, and that Suspense has something to do with async code. So it was just my natural expectation that async components are a thing.

This allows for seamless navigation without flashes of white or loading spinners when content loads fast enough as you will continue to see the old content for the first half a second let's say if the content doesn't load immediately. Only after that time it shows loading state if still hasn't loaded. This has the uncanny effect of making the app feel way quicker even though technically it's less responsive. I demoed it for several people A/B testing style with random delays without saying what I was doing and when asked the difference they all said the Suspense with maxDuration version was preferable because it loaded faster most of the time. The actual load times were no different.

That's a really nice explanation of what Suspense actually can achieve! It would help to put such basics in the docs. Currently the documentation I found on Suspense is this:

image

which is tough to make sense of without prior knowledge of Suspense.

Overall, I think some more detailed "how to" documents could probably answer many of these questions. Preferably written without assuming in detail knowledge of other frameworks (in my opinion Solid's main entry threshold), but with dedicated "For React Users" sections to highlight similarities / differences to React.

ryansolid commented 4 years ago

Yes! Thanks for brushing away the blinders again. Time to step out from React's shadow. React hasn't even completely delivered the feature. I had written the RFC and all previous references from the point of how can I make a version for Solid etc.. But now past that and at the level of feature parity we currently have, it's time to stop treating everything I've been doing as derivative. Technology-wise it is actually quite different. We need guides etc.. I have been working on the Real World Demo since I intrinsically know that and I think once I show how all the pieces working together in something less trivial I will be spending less time justifying each decision.

trusktr commented 4 years ago

@bluenote10 In my example that you replied to, I didn't use render(), I manually appended the DOM reference to the DOM using appendChild. That should work.

Yeah, if you pass a function that returns a Promise into the render() function, then that's not what render() expects, so that won't work. I used

  const app = document.getElementById("app")
  app.appendChild(await App())

instead of

   const el = document.getElementById("app");
  let app = await App()
  render(() => app, el);

There's definitely a difference there. But I can see how someone could make a mistake with that.

I didn't read all the following conversation, but that's the reason I like working with the DOM nodes directly: I am in charge of them, and know how I am connecting them together, using standard APIs that are common across all applications.


If Ryan allowed async functions as computations, he'd have to figure out how to track dependencies across async JavaScript microtasks. Let me just tell you: it is strictly impossible, because it would require hooking into the internal C++ code of the JavaScript engine internals that aren't exposed to JavaScript.

However, it would be possible if computations could be generator function*s. In that case, the JavaScript implementation would be responsible for scheduling functions to continue executing (rather than the internal JS engine doing the scheduling), and this would make it possible for dependencies to be tracked across microtasks/macrotasks ("engine ticks"). But this would add complexity to the reactive implementation.

trusktr commented 4 years ago

EDIT: Oh wait, I read your example too fast: I do see that what you pass into render() is a function that returns a DOM node. I believe that should work.

It'd be great to look at an example on codesandbox to see what's going on.

ryansolid commented 4 years ago

However, it would be possible if computations could be generator function*s.

Yes I did look into this briefly and async generators. It is interesting but takes some real consideration in embedding a pull system inside a push system. And couldn't make much sense on how to account for the sense of time between parallel computations (based on the same dependencies). They almost need to have their own internal sense of time. It's tricky.

ryansolid commented 4 years ago

I've added some documentation: https://github.com/ryansolid/solid/blob/master/documentation/suspense.md. Hopefully that clarifies things some.

bluenote10 commented 4 years ago

Yes, that is super helpful, really nice!

Since promise components per se are probably impossible from a theoretical perspective, I guess we can close the issue?

ryansolid commented 4 years ago

Yeah I just had left it open so that people could see the link to docs. Good to close now.

alllex commented 2 years ago

The last posted link to docs seems to be not valid anymore.

Here is the new link: https://www.solidjs.com/docs/latest#suspense

titoBouzout commented 2 years ago

see also https://github.com/solidjs/solid/discussions/896