kylecorbelli / redux-token-auth

Redux actions and reducers to integrate easily with Devise Token Auth
MIT License
154 stars 80 forks source link

Custom storage never used #37

Open wa0x6e opened 6 years ago

wa0x6e commented 6 years ago

When setting a custom storage engine vie the config

const config = {
  authUrl,
  storage: myStorageEngine,
  userAttributes: {
    firstName: 'first_name',
    imageUrl: 'image',
  },
  userRegistrationAttributes: {
    firstName: 'first_name',
  },
}

the passed custom storage is never used.

I think this is due to this line

https://github.com/kylecorbelli/redux-token-auth/blob/8c5a8fe573918d406a733ca1b21c0b4349f137ab/src/actions.ts#L133

Since flushGetRequests is only present in react native localStorage, the code always fallback to AsyncLocalStorage, which itself default to window.localStorage.

Shouldn't that ternary operator expression reversed ?

alopes commented 6 years ago

There's something wrong with the npm package.

Set it as

"redux-token-auth": "git+https://github.com/kylecorbelli/redux-token-auth.git",

in your package.json and it will work.

wa0x6e commented 6 years ago

I don't think that's the issue, the code I quoted is from the master branch

alopes commented 6 years ago

Can you confirm that /dist/services/auth.js is the same on both versions?

npm https://registry.npmjs.org/redux-token-auth/-/redux-token-auth-0.19.0.tgz

exports.persistAuthHeadersInLocalStorage = function (headers) {
    authHeaderKeys.forEach(function (key) {
        localStorage.setItem(key, headers[key]);
    });
};

repo https://github.com/kylecorbelli/redux-token-auth/blob/master/dist/services/auth.js#L53

exports.persistAuthHeadersInDeviceStorage = function (Storage, headers) {
    authHeaderKeys.forEach(function (key) {
        Storage.setItem(key, headers[key]);
    });
};
wa0x6e commented 6 years ago

I'm not using npm.

The code is from the master branch here:

https://github.com/kylecorbelli/redux-token-auth/blob/8c5a8fe573918d406a733ca1b21c0b4349f137ab/src/actions.ts#L133

alopes commented 6 years ago

From what I can observe, passing custom storage in the config doesn't work at all with the npm package. That's the issue I'm trying to raise here. Regarding your point with storage.flushGetRequests, I believe you are correct. I had to mock flushGetRequests to get cookies as the storage mechanism. Thanks for pointing that out.