happykit / flags

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

useFlags stuck fetching in standalone build after upgrading from Next 13.5.2 -> Next 13.5.6 #60

Open dcyoung opened 6 months ago

dcyoung commented 6 months ago

We currently use HappyKit in a Next project with the following setup:

    "@happykit/flags": "^3.3.0",
    ...
    "next": "13.5.2",

This project uses a standalone next build running in a docker container. We deploy this same image to multiple tenant envs, using env vars to control things like tenant_id and api keys. Internally, we leverage Next's public runtime config to propagate unique env vars instead of NEXT_PUBLIC... method of baking values into images.

const { publicRuntimeConfig } = getConfig() as {
  publicRuntimeConfig: {
    _MY_PUBLIC_TENANT_ID: string;
    _MY_PUBLIC_HAPPY_KIT_FLAGS_PUBLIC_KEY: string;
  };
};

const FeatureFlagContext = createContext<AppFlags | null>(null);

export const FeatureFlagContextProvider = ({ children }: PropsWithChildren) => {
  const useFlags = createUseFlags<AppFlags>({
    envKey: publicRuntimeConfig._MY_PUBLIC_HAPPY_KIT_FLAGS_PUBLIC_KEY,
    defaultFlags: DEFAULT_FLAGS,
  } as Configuration<AppFlags>);

  const { flags, error } = useFlags({
    user: publicRuntimeConfig._MY_PUBLIC_TENANT_ID
      ? {
          key: publicRuntimeConfig._MY_PUBLIC_TENANT_ID,
        }
      : undefined,
  });
  if (error) {
    console.error(
      "Failed to use flags, falling back to defaults. Error: ",
      error,
    );
  }
  return (
    <FeatureFlagContext.Provider value={flags ?? DEFAULT_FLAGS}>
      {children}
    </FeatureFlagContext.Provider>
  );
};

This setup has worked for us for a while now.

We're now attempting an upgrade from Next 13.5.2 to Next 13.5.6 - the only change in our app like so:

-    "next": "13.5.2",
+    "next": "13.5.6",

After this upgrade, everything is working as expected in a dev environment. However our production standalone and dockerized builds are failing to resolve flags. Debugging the issue in such a production build, I can see

Any help debugging further would be appreciated.

dcyoung commented 6 months ago

@dferber90 friendly bump. This is blocking our upgrade of Nextjs, which has security vulnerabilities at the moment.

dferber90 commented 5 months ago

Thanks for the report! Unfortunately it's hard to help without a reproduction.

One thing I noticed is that you're defining const useFlags = createUseFlags<AppFlags>({ from within a component. This is meant to be defined from outside of a component and exported. You're basically creating a new hook on every render which is then called. Could you try moving const useFlags = createUseFlags<AppFlags>({ to the top-level module scope?

dcyoung commented 5 months ago

Thanks for the report! Unfortunately it's hard to help without a reproduction.

One thing I noticed is that you're defining const useFlags = createUseFlags<AppFlags>({ from within a component. This is meant to be defined from outside of a component and exported. You're basically creating a new hook on every render which is then called. Could you try moving const useFlags = createUseFlags<AppFlags>({ to the top-level module scope?

@dferber90 Unfortunately standalone builds error when referencing Next's runtime config outside of components. This provider wraps the app, and it typically only renders once or twice. While not ideal, the re-rendering has not be an issue to date.

To clarify the problem and rule out this culprit, i reran the code with hardcoded keys and useFlags defined outside the component, removing any use of "public runtime config". I'm still facing the same issues with standalone builds using Next 13.5.6. Code below.

import { createContext, useContext, type PropsWithChildren } from "react";
import { createUseFlags } from "@happykit/flags/client";
import { type Configuration } from "@happykit/flags/config";

const FeatureFlagContext = createContext<AppFlags | null>(null);

type AppFlags = {
  my_flag: boolean;
};

const useFlags = createUseFlags<AppFlags>({
  envKey: "MY_KEY",
  defaultFlags: {
    my_flag: false,
  },
} as Configuration<AppFlags>);

export const FeatureFlagContextProvider = ({ children }: PropsWithChildren) => {
  const { flags, error } = useFlags({
    user: { key: "dev" },
  });
  if (error) {
    console.error(
      "Failed to use flags, falling back to defaults. Error: ",
      error,
    );
  }
  return (
    <FeatureFlagContext.Provider value={flags ?? { my_flag: false }}>
      {children}
    </FeatureFlagContext.Provider>
  );
};
dferber90 commented 5 months ago

I noticed you're manually creating a context which is not expected. Check out the example on https://flags.happykit.dev/demo/context with its source code here https://github.com/happykit/flags/blob/master/example/pages/demo/context.tsx.

Here's a version of your code that's working for me

import { type PropsWithChildren, useState } from "react";
import { createUseFlags } from "@happykit/flags/client";
import { type Configuration } from "@happykit/flags/config";
import { useFlagBag } from "flags/client";
import { FlagBagProvider } from "@happykit/flags/context";

type AppFlags = {
  my_flag: boolean;
};

const useFlags = createUseFlags<AppFlags>({
  envKey: process.env.NEXT_PUBLIC_FLAGS_ENV_KEY,
  defaultFlags: {
    my_flag: false,
  },
} as Configuration<AppFlags>);

const FeatureFlagContextProvider = ({ children }: PropsWithChildren) => {
  const flagBag = useFlags({
    user: { key: "dev" },
  });
  return <FlagBagProvider value={flagBag}>{children}</FlagBagProvider>;
};

export default function Page() {
  return (
    <FeatureFlagContextProvider>
      <Inner />
    </FeatureFlagContextProvider>
  );
}

function Inner() {
  const [x, setX] = useState(0);
  const flags = useFlagBag();

  console.log(flags);

  return (
    <p
      onClick={() => {
        setX((p) => p + 1);
      }}
    >
      cool {x}
    </p>
  );
}

Note that this

dcyoung commented 5 months ago

@dferber90 were you able to get the above working in a standalone build using the latest Next 13? (13.5.6)??

The context workarounds listed previously were introduced in order to leverage runtime environment variables that are not baked into the build. I didn't see an easy route around them. Putting that aside to focus on the underlying issue, here is an even simpler example.

The following code works in dev, but is broken when running standalone Next 13.5.6 build.

const useFlags = createUseFlags<AppFlags>({
  envKey: "MY_KEY",
  defaultFlags: DEFAULT_FLAGS,
} as Configuration<AppFlags>);

export default function FlagPrinter() {
  const flagBag = useFlags({
    user: { key: "dev" },
  });
  return <div>{JSON.stringify(flagBag.flags)}</div>;
}
dferber90 commented 5 months ago

Yes it's working for me. Could you provide a repository which reproduces this? Without a reproduction it's very hard to help here