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

feat: add isClient guard #183

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

juliusmarminge
Copy link

Hey there,

Small little utility guard for checking if a client is a PlanetScale client. We currently have a helper like this in our codebase

function isPlanetScaleClient(client: unknown): client is PlanetScale.Client {
  return (
    Predicate.isRecord(client) &&
    Predicate.isRecord(client.config) &&
    Predicate.isFunction(client.transaction) &&
    Predicate.isFunction(client.execute) &&
    Predicate.isFunction(client.connection)
  );
}

due to some issues with instanceof check unexpectedly returning false.

Accept or deny as you wish :)

@ayrton
Copy link
Contributor

ayrton commented Sep 6, 2024

@juliusmarminge thank you for your contribution.

Can you explain a little what the use case is for checking whether the client is a PlanetScale client, and in which instances instanceof would not work?

My best guess is that this is for narrowing, if we accept this patch we'll want to document that this utility is available, what/when it should used for.

@ayrton
Copy link
Contributor

ayrton commented Oct 14, 2024

For future ref Markdoc does something similar but as a static method on the class

@juliusmarminge
Copy link
Author

juliusmarminge commented Oct 14, 2024

Hey, sorry your other comment must have passed by my inbox.

We need to check if a client is a Client or mysql2.Pool. We then create a drizzle client based on that. This is cause we allow both to run on serverless (where we wanna use your http driver) and on stateful servers (where we want to use persistent tcp connection).

export class MySqlDrizzle extends Context.Tag("@uploadthing/db/Client")<
  MySqlDrizzle,
  DatabaseClient
>() {
  static affectedRows = (result: QueryResult["type"]): number => {
    const row = Array.isArray(result) ? result[0] : result;

    const count =
      "rowsAffected" in row
        ? row.rowsAffected // pscale
        : row.affectedRows; // mysql2

    return count;
  };

  static make = (
    client: mysql2.Pool | mysql2.Connection | PlanetScale.Client,
  ): DatabaseClient => {
    if (isPlanetScaleClient(client)) {
      return drizzlePlanetscale(client);
    } else {
      return drizzleMysql2(client);
    }
  };
}

Not sure when client instanceof Client failed, but it did in one case and that's why i created this PR. Maybe we had multiple versions of the lib bundled, who knows xd. Understand if you don't wanna move forward with this without a reproduction though.

effect also uses function for guards that checks the presence of a symbol instead of relying on prototype chain (e.g. https://github.com/Effect-TS/effect/blob/9e2cc4eafa16dae21022904142132ddbe307f785/packages/effect/src/Micro.ts#L136)

lmk how you wanna move forward

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