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

Set strict kysely peer dependency version range #32

Merged
merged 3 commits into from
Oct 9, 2023
Merged

Set strict kysely peer dependency version range #32

merged 3 commits into from
Oct 9, 2023

Conversation

omikader
Copy link
Contributor

kysely version 0.26.0 introduced a breaking API change that removed the orWhere method which is used in the PostgresIntrospector class. See #28 for more context.

There are two approaches for fixing this issue.

Set a stricter peerDependency on kysely

  • Forces users of kysely-data-api to use a kysely version with a compatible API
  • Bump a patch version on kysely-data-api
  • Maybe yank older releases that don't have this constraint?
  • Then, in a separate PR, upgrade kysely-data-api to use ^0.26.0 along with a major version bump to 1.0.0

The pros of this approach are that we don't have to do any type gymnastics. However, this could potentially be disruptive for people who are happily using kysely-data-api on ^0.26.0 without issue because they are not depending on the PostgresIntrospector class.

Introduce a runtime typecheck

As per the linked issue, this could look something like this:

.where(
(qb) => typeof qb.where === 'function' 
  ? qb.where("c.relkind", "=", "r").orWhere("c.relkind", "=", "v") 
  : qb("c.relkind", "=", "r").or("c.relkind", "=", "v")
)

However, in order for this to pass the type system, we'd need to add some utility functions with specific type guards which can get a bit messy.

function qbHasWhere(qb: any): qb is { where: (condition: string, operator: string, value: string) => any } {
  return typeof qb.where === "function";
}

function qbIsCallable(qb: any): qb is { (condition: string, operator: string, value: string): any } {
  return typeof qb.where === "undefined";
}

...

.where(
(qb) => {
  if (qbHasWhere(qb)) qb.where("c.relkind", "=", "r").orWhere("c.relkind", "=", "v") 
  else if (qbIsCallable(qb)) qb("c.relkind", "=", "r").or("c.relkind", "=", "v")
  else // oops
}

We could also sprinkle in some // @ts-ignore but I'd rather avoid that if possible.

@changeset-bot
Copy link

changeset-bot bot commented Sep 19, 2023

🦋 Changeset detected

Latest commit: c0e1358

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
kysely-data-api Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@thdxr thdxr merged commit 7f5ebdd into sst:master Oct 9, 2023
@github-actions github-actions bot mentioned this pull request Oct 9, 2023
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