search

Dan6erbond / sk-auth

Authentication library for use with SvelteKit featuring built-in OAuth providers and zero restriction customization!
MIT License
578 stars 70 forks source link

Properly export and import the `mergePath` helper #75

Closed ohmree closed 2 years ago

ohmree commented 2 years ago

Fixes #74.

Until this gets merged it's possible to use my branch:

$ npm rm sk-auth && npm i ohmree/sk-auth#fix-mergepath # or yarn, or pnpm
mitjakukovec commented 2 years ago

When can we expect this to be merged?

vhscom commented 2 years ago

I tested this locally and, while I was able to call signOut it's not clear how to use it properly. @ohmree do you have an example of using signout once it's available? I see in the source code it appears this feature is about one-half baked as there's a commented out session import at the top of the page. I'm guessing @Dan6erbond was attempting to model from the NextAuth signout module, had to stop, then decided not to export signOut on purpose since the feature isn't complete. What has your experience been?

ohmree commented 2 years ago

What has your experience been?

I actually don't remember, I've since switched to a spa with supabase for auth.

Sorry I couldn't be more helpful.

fusepilot commented 2 years ago

I've read the comments, but don't yet understand the hold up for getting this merged. signOut.ts calls a function named mergePath here and here, but mergePath is not defined anywhere in the file. As a result, I get this runtime error: ReferenceError: mergePath is not defined when I call signOut.

This PR fixes it for me. Would love to see it merged.

vhscom commented 2 years ago

@fusepilot did you try implementing signOut after the helper function is fixed? If not, the current approach of failing loudly is more obvious than a helper function which cannot be implemented properly. However, if you got sign out working with the helper fixed it would be useful to know your implementation for sign out.

Dan6erbond commented 2 years ago

@vhscom the sign-out functionality is something I have to look into. I'm unsure at the current time if it's done or not, but I believe that the mergePath helper should be made available either way, as the "loud fail" is an unintentional warning more than anything else. I'll look into the other issue soon, for now this will be mergerd.