Skip to content

fix: #24 Next.js SDK bad URL defaulting #135

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Draco1js
Copy link

@Draco1js Draco1js commented Jul 2, 2025

Fixing the issue in #24 , should be a bit less error prone now


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Contributor

@thomasballinger thomasballinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this doesn't work, in JavaScript passing undefined uses the default value just like not passing the variable!

tomb@macbookpro convex % node
Welcome to Node.js v18.20.6.
Type ".help" for more information.
> function foo(a=1) { return a; }
undefined
> foo(3)
3
> foo()
1
> foo(undefined)
1

I had a version of this in that caused a problem with Convex Auth, we could merge that again if we can confirm the Convex Auth issue is fixed?

@Draco1js
Copy link
Author

Draco1js commented Jul 5, 2025

I apologize, I thought I tested it pretty well the other day, can you see if you can break this function any way?

function getConvexUrl(
  deploymentUrl?: string | undefined
) {
  if (arguments.length === 0) {
    deploymentUrl = "ENV FALLBACK";
  }
  else if (deploymentUrl === undefined) { // Should skip over this if it hits the first one
    return `deploymentUrl is undefined, are your environment variables set?`;
  }

  return deploymentUrl;
}

console.log(getConvexUrl(), getConvexUrl(undefined), getConvexUrl("hello"));

This is what the output should look like:

ENV FALLBACK
deploymentUrl is undefined, are your environment variables set?
hello

@thomasballinger
Copy link
Contributor

thomasballinger commented Jul 5, 2025

Cool, and thanks to get-convex/convex-auth#156 looks like the problem I had last time has been addressed. I think we need to roll it out carefully though since until now it was possible to pass undefined to mean "use the default." Could you make this console.error for now add a note to to the options documentation at

/**
* The URL of the Convex deployment to use for the function call.
* Defaults to `process.env.NEXT_PUBLIC_CONVEX_URL`.
*/
url?: string;
that you should not pass undefined here, that passing undefined will throw an error in the future? And we'll make it an error in a few versions, probably the next minor version bump that requires explicit upgrade instructions so we can call it out.

It would be nice to add a TypeScript error when passing undefined explicitly but we can't expect https://www.typescriptlang.org/tsconfig/#exactOptionalPropertyTypes to be on for everyone so let's not.

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

Successfully merging this pull request may close these issues.

2 participants