lostpebble / pullstate

Simple state stores using immer and React hooks - re-use parts of your state by pulling it anywhere you like!
https://lostpebble.github.io/pullstate
MIT License
1.08k stars 23 forks source link

Export `PullstateInstance`? #5

Closed bitttttten closed 5 years ago

bitttttten commented 5 years ago

Is it possible to use PullstateInstance as a type? I am making a function that gets passed all my stores, so I want to say:

type AllStores = PullstateInstance<{
    PlaceShareLinksStore: Store<IPlaceShareLinksStore>
}>

export type Hook = (
    toRoute: IRoute,
    toState: TRouteState,
    instance: AllStores,
) => Promise<void>

so then I can use this type for a function, like 'onEnter' where onEnter: Hook and is used like so:

        onEnter: async (_, state, Store) => {
            const { data } = await fetchPlacesFromShareId(state.id)
            await Store.stores.PlaceShareLinksStore.update((s: any) => {
                s.data = data
            })
        },

I am still learning Typescript so maybe there is a better way to handle this, although this is the only way I can see making sense so far!

lostpebble commented 5 years ago

Hi there!

Interesting use case.

I've added an export for IPullstateInstanceConsumable in the latest version 0.8.0-alpha.2, please check it out and see if it helps, and let me know. It basically exports all the "public" API available on PullstateInstance so you can hopefully use it for your use case there.

Btw, have you looked into AsyncActions yet? Its an entire thing I've been building out the past month and a bit pretty much exactly for these kinds of scenarios.

I need to update the documentation on it soon because somethings have changed to make them even more dynamic (cache busting and post-action hooks), but the basics of them are available in the Readme here.

Once you've made an action (e.g. placesFromShareIdAction) you could do something along the lines of a simple:

onEnter: async (_, state, Store) => {
    await placesFromShareIdAction.run({ id: state.id });
},

Which will run and update your store as you have defined in your Async Action. Its also automatically cached per the state.id that's passed in which can be great for performance. But you need to use the new Async Action hooks to make use of that cached value each time, specifically the postActionHook - you can see a basic explanation here (need to document properly soon). -for now you can see from the TypeScript types how they function, you pass the hooks in as a second argument to the Async Action creator.

EDIT:

Actually, running await placesFromShareIdAction.run({ id: state.id }); directly like that causes your action to run fully, because its a direct "run" of your action (so ignore what I said about the cache above). If you go the postActionHook way, and want to hit a possibly cached result instead of running the full action, you need to pass an option as the second argument there:

await placesFromShareIdAction.run({ id: state.id }, { respectCache: true });
bitttttten commented 5 years ago

IPullstateInstanceConsumable works, thanks 👍

Yeah, my use case is because I didn't want to go down the route of rendering twice on the server. So I have all my actions declared in hooks on the router level. Then I am able to get the matched route, call the hook, and then render my app (I outline it more in my docs for my router library here).

I only just started with pullstate, and haven't gotten onto AsyncActions yet as I was still taking the idea in haha. This is an easy update to migrate into actions, and I am very curious to get into them. The respectCache seems very nice and usually I did that myself with mobx and immer stores but it was a lot of boilerplate or copy and pasting.

Thanks for the detailed reply :)

lostpebble commented 5 years ago

Great! Glad to hear its working out for you.

I'm in the process of putting together a proper documentation site for pullstate. I'll hopefully have it done by this weekend and it'll detail all the Async Action stuff a lot better.

That's an interesting way to get around the double render problem btw. Its an unfortunately problem, and the way Async Actions currently handles it is with multiple renders (seems its the only way to get your hooks to register, and hence register which async state needs to be resolved). I'm hoping React provides a better way to do this soon, as I think they mentioned something about it with their new react-cache (their native async state handling) stuff.


Your way of solving it with routes has me thinking though, Async Actions should allow you to "pre-resolve" (pre-cache) your async state before the first render, which essentially means it won't need to render twice... and generally async state is pretty much route based and only needs to resolve once per route.

Currently Async Actions only allows you to resolve async state during render (it uses the stores passed down in <PullstateProvider>), but I think I should actually put the ability to run Async Actions outside of the render cycle - on PullstateInstance, so we can easily run them on our server routing outside of React and pre-cache their results.

I think this is actually what you're trying to do? If so, it seems that Async Actions don't work yet for your scenario - and you'll have to update the stores in PullstateInstance directly still. But I'll definitely be adding that in now!

Thanks for the interesting use case :)

bitttttten commented 5 years ago

I think this is actually what you're trying to do

Yep, I think it's exactly what I am trying to do! Then I will just keep reading on Async Actions :) It would be nice to be able to run them outside of the render cycle.. but I don't know how complicated that is for you. Also perhaps it moving in the opposite direction of async react so you might want to think about that before doing it just for me ;)

In the future, I would move more in a direction of async react, but that direction is still unclear so I am following this pattern of keeping shared client/server data out of my routes so it's simpler to work with.

Edit: I think I will be using Async Actions for the more complicated client-side only state.

lostpebble commented 5 years ago

Also perhaps it moving in the opposite direction of async react so you might want to think about that before doing it just for me ;)

No worries, I also don't really want to wait for that - and when they do finally have a solution it should require no changes (the async state will still be cached in pullstate so we shouldn't have to run the actions again - no matter the process given by React. I would have to find a way for pullstate to do this with their new process either way).

Also, the benefit of this library being quite small is that its easy to iterate on these kinds of ideas. :)

I'll definitely be taking a look into it soon, as its a great use case (rendering twice hurts my soul haha).

bitttttten commented 5 years ago

Well, fair enough then! I will keep an eye on the progress of async actions :)

bitttttten commented 5 years ago

Whilst we are on the topic though, what is so bad about rendering twice? I wanted to avoid it since it was just cleaner and easier to reason about, but perhaps it's not a bad choice since it's easier to set up, less code, and you wouldn't need 2 entry points for your app.. maybe the performance is negligible.

lostpebble commented 5 years ago

It's not all that bad. But I've had some large React apps that can take up to 50 - 70ms to render, and every millisecond spent rendering is one millisecond slower for the user. If I can, I'd like to optimize things as much as possible - as its small things like this which build up to sometimes large performance hits at the end.

If you think about rendering React trees, its a lot of code at times - functions for hooks, mounting things, potentially some data transformations while rendering a list of items etc. It's all a cost on the server CPU - and doing that more than necessary doesn't seem great. If you think about it - you could save half the amount of compute power, and use less machines / serve more traffic.

Though, as you say, there are times where it is probably negligible (I'm doing it in a couple small apps and its seriously not a problem at all) - but when building tools which could be used with many applications I think its best to try provide ways the user can optimize their app as much as possible for when it potentially could become a bottleneck.

less code, and you wouldn't need 2 entry points for your app..

Yea the bummer is duplicated code now to pre-fetch things before render. But unless you have a very complex app, something conceptually simple like url -> asyncActionsToRunBeforeRender[] might do the trick.

lostpebble commented 5 years ago

Just to update you, I've added the ability to run Async Actions on the instance now - allowing us to pre-fetch Async Action state before render, which will be added to the cache and pullstate will then see it and not indicate that further renders are required.

The code for one of my internal apps now looks like so (I use koa and koa-router for route matching):

Put the pullstate instance into the current context:

ServerReactRouter.get("/*", async (ctx, next) => {
  ctx.state.pullstateInstance = PullstateCore.instantiate({ ssr: true });
  await next();
});

Create the routes, which run the various required actions:

ServerReactRouter.get("/list-cron-jobs", async (ctx, next) => {
  await ctx.state.pullstateInstance.runAsyncAction(CronJobAsyncActions.getCronJobs, { limit: 30 });
  await next();
});

ServerReactRouter.get("/cron-job-detail/:cronJobId", async (ctx, next) => {
  const { cronJobId } = ctx.params;
  await ctx.state.pullstateInstance.runAsyncAction(CronJobAsyncActions.loadCronJob, { id: cronJobId });
  await next();
});

ServerReactRouter.get("/edit-cron-job/:cronJobId", async (ctx, next) => {
  const { cronJobId } = ctx.params;
  await ctx.state.pullstateInstance.runAsyncAction(CronJobAsyncActions.loadCronJob, { id: cronJobId });
  await next();
});

And render the app:

ServerReactRouter.get("*", async (ctx) => {
  const { pullstateInstance } = ctx.state;

  // render React app with pullstate instance

What's cool about this is that the Async Actions you use on the server and the ones you use on the client are exactly the same - so its really nice for server-rendered SPAs. Everything just runs and caches as needed. You could even pre-cache a few pages on the server at once if you like (depending on how big you want the initial page payload to be), and have instant page changes on the client (and Async Actions has custom cache busting built in to invalidate async state which is too stale - such as the user taking too long to change the page).

EDIT: Again, thanks for bringing this up! I think this will bring a nice little performance boost on some of my bigger apps.

This is now available on 0.8.0-alpha.3

lostpebble commented 5 years ago

Hi @bitttttten

Just to let you know, I've almost finished creating a documentation site for Pullstate. The section which might interest you is right at the end: https://lostpebble.github.io/pullstate/docs/async-server-rendering

Hope thinks are working out on your side!

bitttttten commented 5 years ago

Hey!

It's working very well, although I am only looking at the client side code at the moment. I haven't tested SSR yet :)

It's really nice to be able to run actions without the overhead of a store too.

bitttttten commented 5 years ago

Although I just hit this error:

Uncaught TypeError: Cannot read property 'hasOwnProperty' of undefined
    at checkKeyAndReturnResponse (1.chunk.js:36229)
    at useWatch (1.chunk.js:36389)
    at Object.useBeckon (1.chunk.js:36402)

cache.results seems to be undefined in useWatch.

What I am doing is:

import { errorResult, successResult, createAsyncAction } from 'pullstate'

export const getUser = createAsyncAction(
    async function getUser({ userId }: { userId: string }) {
        try {
            const response = await _getUser(userId)

            if (responseIsValid(response)) {
                return successResult(response.data)
            }

            return errorResult(
                [],
                `Couldn't get user: ${getTechnicalMessageFromResponse(
                    response,
                )}`,
            )
        } catch (e) {
            return errorResult(
                [],
                `Couldn't get user: ${getTechnicalMessageFromError(e)}`,
            )
        }
    },
)

my instance:

export const Store = createPullstateCore({
    PlaceShareLinksStore,
})

export const getUserAction = Store.createAsyncAction(getUser)

and in my component:

import { getUserAction } from '../../stores'

const [finished, result] = getUserAction.useBeckon({
    userId: state.id,
})

I am only rendering on the client so far, so there is no initial state. It's rendering through CRA.

lostpebble commented 5 years ago

Hi @bitttttten

Okay I just checked out some of your code and its seems strange that an error is popping up. I tried to replicate the exact example in a code sand box, and it seems to work:

Edit currying-shape-jn0ng

It might have something to do with how you're creating it.

I see you said you're creating the action like so:

export const getUser = createAsyncAction(

But then later I see you're passing the already created action into another action creator:

export const getUserAction = Store.createAsyncAction(getUser)

I think that may be the issue.

As some extra info, because you're creating a pullstate "Core" for your stores, you should create your actions only using that. This will let you transition over to SSR very easily when its time. So use Store.createAsyncAction(async () { // action code }, optionalHooks) in your case.

lostpebble commented 5 years ago

Btw, I can see where the confusion can come in if that is the case. Pullstate provides two ways to create actions because some people don't care about SSR, or sometimes you just want to create a simple Async Action which doesn't touch your stores (like image loading before fading in) - for those client-only actions you can create them directly with the createAsyncAction import, for all others you should use your created Pullstate core.

bitttttten commented 5 years ago

My example code may not be up to date, I may have an error in copy and pasting it over. Since I was using actions in the non-SSR way first, just to understand the syntax, and then migrating over to the SSR friendly way.

So this line:

export const getUser = createAsyncAction(

should be:

export async function getUser(..)

I will share the detailed code tomorrow, as it's on my laptop!

Also with this way, I just need to define actions.. and not have to deal with any stores. Right? If I understand it correctly then it is a lot cleaner as sometimes a store is such an overhead and conceptually you might use your smaller stores for just one or two actions.

lostpebble commented 5 years ago

Yep completely! If you simply want some asynchrounous state to be trackable and usable in your React components, you can create the Async Action to return the result you want directly - and you don't need to touch your stores or the Pullstate "Core" at all.

I've updated the example I posted with code that does just that.


In fact the proper way to use Async Actions with your stores involves passing hooks which run at certain times, specifically a postActionHook which will run consistently on a cached return value and a first-time async run (so you can do post-action data changes consistently in your stores). It really depends how much you want to rely on Pullstate for state.

You can see more about hooks here: https://lostpebble.github.io/pullstate/docs/async-hooks-overview

I think they're part of what make async actions so powerful, allowing you to break the cache depending on the current result, run things consistently in your stores after actions, and also a "short-circuit" hook which allows you to return a result early without actually running the full action.

Although you very well could use the basic async actions without worrying about any of that, they're simply there to allow for more complex async situations when the time comes :)

bitttttten commented 5 years ago

Yeah, the cache breaking is a really nice feature. I kind of wanted an API that gives it to you for free. Handling logic so that the client doesn't have to check if the server already called the method was a bit of a pain!

I will share with you my code, which is currently working but only if I use useBeckon. I would like to use useWatch so it doesn't trigger the action on the client again. Maybe it's because I am on 0.8.0-alpha.3?

This is my route, from the oatmilk library I referenced earlier. onEnter is called before rendering on the server, although it runs again on the client.. even though the fingerprint is the same.

const route: IRoute<OatmilkHook> = {
    path: '/user/:id',
    name: 'user',
    view: UserPage,
    onEnter: async (_, state, instance) => {
        await instance.runAsyncAction(getUserAction, {
            userId: state.id,
        })
    },
}```

This is my Store/instance:

```js
export const Store = createPullstateCore({
    PlaceShareLinksStore,
})

export const getUserAction = Store.createAsyncAction(getUser)

And this is how I use it:

export default function UserPage() {
    const { state } = useOatmilk()
    const [finished, result] = getFlamyngoUserAction.useBeckon({
        userId: state.id,
    })

    if (!finished) {
        return null
    }

    return (
        <UserProvider
            firstName={result.payload ? result.payload.firstName : ''}
            lastName={result.payload ? result.payload.lastName : ''}
            id={result.payload ? result.payload.id : ''}
        >
            {..}
        </UserProvider>
    )
}

And just so it helps, here is my initial data:

window.INITIAL_DATA = {
  allState: { PlaceShareLinksStore: { data: {} } },
  asyncResults: {
    "0-{userId~MY_USER_ID~}": [
      true,
      true,
      {
        payload: {
          id: "MY_USER_ID",
          firstName: "TEST",
          lastName: "TEST",
        },
        tags: [],
        message: "",
        error: false
      },
      false
    ]
  },
  asyncActionOrd: { "0-{userId~MY_USER_ID~}": 0 }
};

And it's working very well for me apart from the client side action being run again.

I have not had a case where I need hooks yet.. and in this case, I do not want to break the cache since it would be a waste of client-side resources.

lostpebble commented 5 years ago

Okay cool. So looking at your code, useBeckon should not trigger the action again - but only if your pullstate instance is being hydrated properly on the client-side. If it can't see that state from the server (find the already cached result for those arguments), it will trigger the action again.

How are you hydrating that state in window.INITIAL_DATA?

For server-rendering you need to make use of <PullstateProvider> with the hydrated instance of your stores passed in:

  const hydrateSnapshot = JSON.parse(window.__PULLSTATE__ || "null", SerializationUtils.JsonRevivers.reviveDateObjects);
  const instance = PullstateCore.instantiate({ ssr: false, hydrateSnapshot });

  ReactDOM.render(
    reactTreeFunction(
      () => (
        <PullstateProvider instance={instance}>
          <App />
        </PullstateProvider>
      ),
      { styleProviderProps: { renderer: getVibescoutEditorRenderer() }, routerProps: { history: browserHistory } }
    ),
    document.getElementById("react-mount")
  );

That's what my client-side entry looks like on one of my projects.

This allows useBeckon() to use the Pullstate context provided by <PullstateProvider> and recognise that the action has been resolved already.


Something off-topic to note there is the function passed to JSON.parse() to revive date objects, reviveDateObjects, as JavaScript can't tell the difference between them and a regular string otherwise - I'm happy to share the code I snatched off StackOverflow at some point, if you like :)

lostpebble commented 5 years ago

I just realized that because you are running your async actions outside of the render, you can actually hydrate Pullstate without using the <PullstateProvider>.

You simply need to make sure you instantiate your PullstateCore before your first render on the client, like so:

const Store = createPullstateCore({
    PlaceShareLinksStore,
});

const hydrateSnapshot = JSON.parse(window.INITIAL_DATA || "null", reviveDateObjects);
Store.instantiate({ ssr: false, hydrateSnapshot });

Internally this puts those cached results from the server into the client cache.


Actually.. scrap that.

I think what you're trying to do may seem to work on the server at the moment, because the server is treating your stores as if you are on the client side. This is dangerous as it means that all requests are using the same state stores from where they last left off (the previous renders) - when really you want them to be re-newed and in the default state and run fresh actions from the start of each request.

How are you instantiating your Pullstate core (Store in your case) on the server?

For server-rendering, which you are doing because you need unique state per request which changes your app rendering from the server, you will need to use <PullstateProvider> and instantiate your "Core" correctly on the server, i.e you need to do this:

const instance = Store.instantiate({ ssr: true });

// render react like so:
<PullstateProvider instance={instance}>
   <App />
</PullstateProvider>

This allows useBeckon() inside your React tree to alter the cache inside this unique request - instead of the "global" cache which is reserve for the client-side only.

In your case, you have pre-run those actions like so:

await instance.runAsyncAction(getUserAction, {
   userId: state.id,
})

But you still need to use <PullstateProvider> as its the only way to provide your pre-run action's state to your app during rendering. If you didn't put that instance into the provider, useBeckon() can't see the result and will have queued up another run the regular way (multiple renders required).

I can't know for sure looking at your code right now, but I think this may be the issue.

lostpebble commented 5 years ago

Updated my comment with some more info about your use-case.

bitttttten commented 5 years ago

Thanks for the detailed reply!

So I refactored my code to only use actions, I just have 2 actions now and stores seemed like a bit of an overhead. Also I like the idea of useBeckon inside my components, so I can prefetch and preload other components using react's hidden boolean.

Although, reading your post and checking my code it seems like I am following all the steps correctly. However, you are right. It seems that the call to the API is run on the client too.. it's not deal breaking however it does tell me that perhaps I have something set up incorrectly.

A Provider on the server and client have been there all along, I didn't change or remove them.

Here is my client entry code:

const hydrateSnapshot = window.INITIAL_DATA
    ? window.INITIAL_DATA
    : { allState: {} }

// allow the passed state to be garbage-collected
delete window.INITIAL_DATA

const instance = Store.instantiate({ ssr: false, hydrateSnapshot })

function App() {
    return (
        <PullstateProvider instance={instance}>
            <App>
                <oatmilk.RouterView />
            </App>
        </PullstateProvider>
    )
}

loadableReady(() => {
    const root = document.getElementById('root')
    if (root && root.hasChildNodes()) {
        ReactDOM.hydrate(<Index />, root)
    } else {
        ReactDOM.render(<Index />, root)
    }
})

This is on the server:

    const instance = Store.instantiate({ ssr: true })

    // look below for what a route looks like..
    await currentRoute.onEnter(route, state, instance)

    const jsx = React.createElement(
        PullstateProvider,
        { instance },
        React.createElement(App),
    )

Then I have a route that looks like:

const route = {
    path: '/places/:id',
    name: 'places',
    view: PlacesPage,
    onEnter: async (_, state, instance) => {
        if (!state || !state.id) {
            return
        }
        await instance.runAsyncAction(fetchPlacesFromShareIdAction, {
            shareId: state.id,
        })
    },
}

If you remember that onEnter is called on the client and server. So when I run await currentRouter.onEnter I am running the above which awaits on the async action.

My "store" looks like this now:

export const Store = createPullstateCore({})

export const fetchPlacesFromShareIdAction = Store.createAsyncAction(
    fetchPlacesFromShareId,
)

I am more than happy to share more details and code with you! Plus I have some other questions about the best practises for testing and so on if you don't mind, of course. Are you on the reactiflux Discord?

lostpebble commented 5 years ago

Hi @bitttttten

Sorry, I've been off for a bit of a break this past week - but I just got back now. Will take a look into this in more detail tomorrow.

And yep, I'm on the reactiflux Discord - you can find me as lostpebble (#7413). I'm not completely clued up on Discord yet, but I think that should suffice?

lostpebble commented 5 years ago

On a quick look at your code today, what happens if you remove this line:

// allow the passed state to be garbage-collected
delete window.INITIAL_DATA

I think what may be happening is that this is deleting the state altogether - because JavaScript assigns references to the same object, not copies, so you could be deleting the data on:

const hydrateSnapshot

I'll try look into it closer now, but that's my first possible red flag.

EDIT:

Its very strange that it isn't working - if removing that delete line doesn't fix it. I'm looking at your code and it seems like it should be working just fine as long as you are using the same instance and then putting that snapshot into window.INITIAL_DATA in the rendered HTML - and placing that snapshot into the client <PullstateProvider>. It it all looks like you're doing that.

I guess I will need to see a small example of your code to really get to the bottom of it, and re-asses if my documention is completely missing something 😅

lostpebble commented 5 years ago

Resolved during our discord chat (although far deviated from the original issue...) - will close for now.

Feel free to open a new issue on this latest discussion if its not completely resolved yet! (or chat on discord again).