happykit / flags

⛳️ Feature Flags for Next.js
https://happykit.dev
MIT License
1.01k stars 11 forks source link

Using useFlags once per page #42

Closed richardantao closed 1 year ago

richardantao commented 1 year ago

From the documentation:

useFlags: This hook should only ever be rendered once per page.

I'm just wondering what is the point of having a hook that can only be called once per page. If this is the case, then API consumers are forced to call useFlags at the top of the component tree, and then would have to either: 1) Pass the flag down the component tree via prop drilling 2) Create our own custom context provider/hook as a wrapper

Doesn't this kill the benefits of using hooks?

Is there any way to dedupe requests - similar to how swr does it - so that the hook can be called multiple times without making additional requests to the API?

Alternatively, would it make more sense for the library to have a context provider called in the _app component so that the hook could be called multiple times?

dferber90 commented 1 year ago

then API consumers are forced to call useFlags at the top of the component tree

This is exactly the intended outcome. The top of the component tree (aka the page component) is the place where the rendering strategy is known (static page, ssr, csr, ..). And since happykit works with all those strategies, it's the best place to call useFlags so that it can do the right thing for each strategy.

Calling it once at the top of the component tree of each page ensures no component makes any assumptions about the rendering strategy, and each component instead simply accepts the flags as props or reads it from context.

The useFlags hook is doing a bunch of things that only need to be done once per page. It's wasteful and unnecessary to do the work multiple times per page. You can share the result by passing it down via props or by introducing a Flag Context. An example of that is shown on the linked page.

Doesn't this kill the benefits of using hooks?

No. Nothing in the contract of hooks says that they need to be idempotent.

Hooks are functions, and calling a function in one place or all over your component tree has different effects.

Is there any way to dedupe requests - similar to how swr does it - so that the hook can be called multiple times without making additional requests to the API?

Yes, you need to call useFlags() at the root of your component tree. And then you render a <FlagBagProvider /> as shown here. You still need to do this on every page as each page might use a different rendering strategy. But you could also do it in _app if you agree on a specific pattern for your page props.

By calling useFlags only once and then passing the result down via props, Flag Context or manually. You can then call useFlagBag in your components to read the values from the context. But this is just a way to avoid passing props down, you still need to call useFlags() at the page.

Alternatively, would it make more sense for the library to have a context provider called in the _app component so that the hook could be called multiple times?

This is pretty tricky to do as the initial value is not known in _app, so SSR would run with the wrong flag values.

I'm sure something better can be done than what the library is doing at the moment, but I haven't quite found a solution I'm actually happy with. I do hear you that it would be ideal if you could just call useFlags in multiple places - but what would happen if you provide a different input to each useFlags() call? Would you get back different values for the flags in those places? Doesn't that lead to inconsistent state, bugs and more requests than necessary? That's why I find more ergonomic to enforce one useFlags call.

richardantao commented 1 year ago

You make some fair points. However, there are still a few things that are unclear to me.

Calling it once at the top of the component tree of each page ensures no component makes any assumptions about the rendering strategy, and each component instead simply accepts the flags as props or reads it from context.

Why is the rendering strategy so important?

const PageComponent = props => {
  const flagBag = useFlags({ initialState: props.initialFlagState });
}

All useFlags needs to know is whether props.initialFlagState is defined or not. If it is defined, use that as the source of flagBag. If it is null | undefined, fetch the state from the API on the client once the page mounts.

I do hear you that it would be ideal if you could just call useFlags in multiple places - but what would happen if you provide a different input to each useFlags() call

In what scenario would initialFlagState from SSG/SSR be different than what useFlags would fetch client-side? There are only two possible inputs: 1) initialFlagState is defined - can only be one shape, since it is provided from a single source per page (SSG/SSR) 2) initialFlagState is undefined - useFlags knows it needs to fetch the state client-side from the API

Regardless of the data fetching method, the source of the flagBag should be the same, since they are both fetched from the same API endpoint, no?

This is pretty tricky to do as the initial value is not known in _app, so SSR would run with the wrong flag values.

In the case of _app, lots of libraries have Providers which receive an unknown (typed) initial value. Either the value is defined and the provider consumes the value to set the initial state, or the value is null | undefined and a default value is used instead.

I'll close by saying that the FlagBagProvider pretty much serves my request. It doesn't take much for me to adapt the code to fit what I'm asking for, so thanks for the reference. At this point, I'm just curious about the design decisions.

dferber90 commented 1 year ago

All useFlags needs to know is whether props.initialFlagState is defined or not.

It also needs to know the user/traits to evaluate for. If this differs from the values in initialFlagState it needs to refetch. An example of this is here https://flags.happykit.dev/demo/dynamics

In what scenario would initialFlagState from SSG/SSR be different than what useFlags would fetch client-side? There are only two possible inputs

There are more inputs, which is what caused #43 for you. Aside from the initial state, you also can pass the user/traits. If those don't match the values used to determine the initial state, the library knows it needs to fetch the latest flags. It's a hassle to carry this information around to every component so it's better to have one useFlags() call at the root.

which receive an unknown (typed) initial value.

It's a longer explanation of why this is problematic. If we forced the flags prop to always be called the same, then the provider could be rendered once in _app and pick the props from pageProps. You can do this in your application today, but I don't want to force it at a library level as it's less flexible.

Glad the FlagBagProvider is useful to you :+2: