search

kleydon / prisma-session-store

Express session store for Prisma
MIT License
116 stars 18 forks source link

Allow prisma.schema to use Json data type for session data #101

Closed hugil closed 1 year ago

hugil commented 1 year ago

Increases flexibility of storing type safe data from auth serialization

kleydon commented 1 year ago

Hi @hugil - thanks for the fix! Looks like a great improvement.

When I try to run the tests, in src/@types/prisma.ts I encounter:

"Cannot find name 'JsonValue'."

Where does this type live? Can you add it via import to "src/@types/prisma.ts"?

Thanks again for the fix.

hugil commented 1 year ago

Hi @hugil - thanks for the fix! Looks like a great improvement.

When I try to run the tests, in src/@types/prisma.ts I encounter:

"Cannot find name 'JsonValue'."

Where does this type live? Can you add it via import to "src/@types/prisma.ts"?

Thanks again for the fix.

Hey, thanks for spotting that. Missed the import. I've committed it now so the test should run fine!

kleydon commented 1 year ago

Hi @hugil , Looks like some of the tests are still failing... Would you mind running npm run test (and npm run lint) from the project directory, to see what's going on? (I'll be offline for much of the rest of today, but back again tomorrow.)

gLenczuk commented 1 year ago

It would be good to mention in README, that you can still use string datatype. Now it seems like JSON is only available. And tests are failing because now serializer accepts only a string. You can try to extend this to also accept JSON, but I'm not sure how it would behave.

kleydon commented 1 year ago

@gLenczuk - Good point! It does feel like the docs need to be updated, for it to be clear that string data can still be used...

@hugil - How likely is it that you will finish working on this PR? (If you don't continue, I'll probably try to finish it off - but could be a little while; I'm under water here with other work.

Would love to get (both of) your advice re: whether the changes in this PR constitute a breaking change.

I'm unclear on whether the session's data field has always implicitly been json (such that formally typing it as json shouldn't cause trouble for existing projects, existing sessions in existing dbs, etc), or whether it is possible that the data field could (for some people, in some projects) represent non-json string, and result in problems when upgrading this library. What do you think?

kleydon commented 1 year ago

@hugil, @gLenczuk - on reflection, it looks this prisma-session-store issue needs an alternate approach (if it is still a problem; I'm using prisma 4.3.0, with the current version of prisma-session-store, and not encountering any type issues related to string vs. json...). According to Prisma's database features table, not all dbs support json as a type - which suggests that where the db is concerned, we should probably stick with the lowest common denominator (string) even though @hugil I agree, json would provide greater type safety. Going to close this for now, and if you have any better ideas - would love to hear them.