pocketbase / js-sdk

PocketBase JavaScript SDK
https://www.npmjs.com/package/pocketbase
MIT License
2.17k stars 127 forks source link

Should LocalAuthStore use globalThis.localStorage instead of window.localStorage? #293

Closed godmar closed 5 months ago

godmar commented 5 months ago

Right now, LocalAuthStore.js tests for window.localStorage before using localStorage. This means it's not working with, for instance, the node-localstorage polyfill for node.js because that polyfill attaches localStorage to global.

Would it be better to use globalThis instead or is there a reason that the implementation requires an actual window object?

ganigeorgiev commented 5 months ago

No. It is better to remain with window as it is safer in case the user uses the library for regular users auth in a server context, eg. JS SSR (see also the note about Deno since the last time I tested there was window global object):

NB! Deno also supports LocalStorage but keep in mind that, unlike in browsers where the client is the only user, by default Deno LocalStorage will be shared by all clients making requests to your server!