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

[email protected] breaks postgres getTables #28

Open
conorbrandon opened this issue Jul 20, 2023 · 8 comments
Open

[email protected] breaks postgres getTables #28

conorbrandon opened this issue Jul 20, 2023 · 8 comments

Comments

@conorbrandon
Copy link

[email protected] removed "the deprecated orWhere, whereExits etc. methods".

Unfortunately, this breaks the postgres-introspector at line 70, required (obviously) for migrations.

Would it be possible to make a backwards compatible change, something like

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

Or maybe specify the kysely peer dependency should be <0.26.0, instead of any 0.x version?

Otherwise, very much enjoying the library. Thanks!

@dan-turner
Copy link

Hi @conorbrandon, experiencing this also. Did you come up with a workaround?

@dan-turner
Copy link

I downgraded to 0.25.0 to confirm this fixes the error I was getting about qb.where being undefined. Problem is I was also using ParseJSONResultsPlugin which wasn't introduced until 0.26.0 so I'm stuck

@conorbrandon
Copy link
Author

@dan-turner my workaround was to use sed to replace the broken line with the 0.26.0 compatible equivalent 😅. If you're on a Mac, you could try adding this as an npm install script in your package.json. Replace esm with cjs depending on which bundle you're using.

{
  "scripts": {
    "install": "sed -i '' 's/qb.where(\"c.relkind\", \"=\", \"r\").orWhere(\"c.relkind\", \"=\", \"v\")/qb(\"c.relkind\", \"=\", \"r\").or(\"c.relkind\", \"=\", \"v\")/' node_modules/kysely-data-api/dist/esm/postgres-introspector.js"
  }
}

It's not an ideal workaround... The requirements for my project actually changed and I can no longer use Postgres (I was actually going to close the issue, glad I didn't!), so I didn't get a chance to deploy this and/or depending on your build step this might not work, but at least testing locally, it seemed to work.

@omikader
Copy link
Contributor

Would love to get a fix in for this -- I am also version pinning kysely to 0.25.0 in the interim.

@dan-turner
Copy link

@thdxr any thoughts?

@thdxr
Copy link
Collaborator

thdxr commented Sep 18, 2023

will accept a PR

@woconnor
Copy link
Contributor

A single where("c.relkind", "in", ["r", "v"]) clause seems to work - is there a reason not to use this approach?

Install script inspired by @conorbrandon:

{
  "scripts": {
    "install": "sed -i.bak 's/.where((qb) => qb.where(\"c.relkind\", \"=\", \"r\").orWhere(\"c.relkind\", \"=\", \"v\"))/.where(\"c.relkind\", \"in\", [\"r\", \"v\"])/g' node_modules/kysely-data-api/dist/esm/postgres-introspector.js"
  }
}

@iloewensen
Copy link
Contributor

will accept a PR

@thdxr Here is an open pr to hopefully resolve the issue
#44

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

6 participants