Skip to content
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

Next.js SDK bad URL defaulting #24

Open
xixixao opened this issue Dec 16, 2024 · 1 comment
Open

Next.js SDK bad URL defaulting #24

xixixao opened this issue Dec 16, 2024 · 1 comment

Comments

@xixixao
Copy link
Contributor

xixixao commented Dec 16, 2024

The way backend url option is defaulted is error-prone.

Right now:

function setupClient(options: NextjsOptions) {
  const client = new ConvexHttpClient(
    getConvexUrl(options.url, options.skipConvexDeploymentUrlCheck ?? false),
  );
  if (options.token !== undefined) {
    client.setAuth(options.token);
  }
  if (options.adminToken !== undefined) {
    client.setAdminAuth(options.adminToken);
  }
  client.setFetchOptions({ cache: "no-store" });
  return client;
}


function getConvexUrl(
  deploymentUrl: string | undefined,
  skipConvexDeploymentUrlCheck: boolean,
) {
  const url = deploymentUrl ?? process.env.NEXT_PUBLIC_CONVEX_URL;
  const isFromEnv = deploymentUrl === undefined;
  if (typeof url !== "string") {
    throw new Error(
      isFromEnv
        ? `Environment variable NEXT_PUBLIC_CONVEX_URL is not set.`
        : `Convex function called with invalid deployment address.`,
    );
  }
  if (!skipConvexDeploymentUrlCheck) {
    validateDeploymentUrl(url);
  }
  return url!;
}

This means that if I call preloadQuery(api.foo.bla, {}., {url: process.env.MY_VAR}) when MY_VAR is not used the code falls back to NEXT_PUBLIC_CONVEX_URL. It would be better to check whether 'url' in options and default based on that, throwing early if the provided url is undefined.

Fixing this would unfortunately be a breaking change in the strict definition of breaking.

@thomasballinger
Copy link
Contributor

We'll call it out in the release and make this change, this is just better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants