sst / kysely-data-api

75 stars 21 forks source link

kysely@0.26.0 breaks postgres getTables #28

Open conorbrandon opened 1 year ago

conorbrandon commented 1 year ago

kysely@0.26.0 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 commented 1 year ago

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

dan-turner commented 1 year ago

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 commented 1 year ago

@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 commented 11 months ago

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

dan-turner commented 11 months ago

@thdxr any thoughts?

thdxr commented 11 months ago

will accept a PR

woconnor commented 11 months ago

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 commented 1 month ago

will accept a PR

@thdxr Here is an open pr to hopefully resolve the issue https://github.com/sst/kysely-data-api/pull/44