tolitius / inquery

vanilla SQL with params for Clojure/Script
17 stars 5 forks source link

CLJS version is node.js only #1

Open vnorilo opened 3 years ago

vnorilo commented 3 years ago

Hello and thanks for publishing this library!

I was trying to use it in a web app that is a front end for making SQL queries, but it pulls in node.js and thus doesn't work in the browser.

Upon investigation, looks like this is solely due to inquery.core/read-query which reads the local file system.

I would love to move read-query and make-query-map to a separate namespace so that a client who doesn't need them can avoid pulling node.js. This would break clients, but only so far as they'd need to :require this additional namespace if they use make-query-map.

I'm happy to provide a pull request if you think this would be workable.

Thanks again, Vesa

tolitius commented 3 years ago

Hi Vesa, thanks for the idea!

I am a bit torn, since I know that many apps are using this lib and all will have to update in case these fns are moved to a different namespace At the same time your approach makes a lot of sense and does make things better going forward so I am leaning towards: we should do it.

feel free to create a pull request decoupling I/O from the SQL creation one thing I would ask is to include tests to make sure it still works as documented