-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
async storage -> community #4027
Conversation
🤤 |
Please merge this! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved, correctly replaces the deprecated library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved that this change works as expected with RN >=0.60
When can we expect this to go through? I have many projects that aren't keeping my users credentials because of this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix #4272
Would love to see this merged as well. |
Can we get an AWS Rep to review this changes asap? |
All these millions on Amazon Any help AWS pals? 🤷♂ |
@sospedra it's already deprecated for people with RN version >=0.60 and the async storage functionality doesn't work for us |
Good to know. |
Any chance someone from AWS can look at this? This is preventing me from releasing. We can't release when user session is not being persisted. This PR has been open for 7 weeks. |
Temporary solution
|
Is this not being merged because there is a merge conflict? If so, @jpaas can you update your PR? |
I think a fix has been merged? 1207227 |
@@ -67,6 +67,7 @@ | |||
"js-cookie": "^2.1.4" | |||
}, | |||
"devDependencies": { | |||
"@react-native-community/async-storage": "^1.6.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe can update to latest version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jpaas can you resolve this?
In case you need a workaround solution for this ASAP, you can see my code here: import {Auth} from 'aws-amplify';
import AsyncStorage from '@react-native-community/async-storage';
import {ICognitoStorage} from 'amazon-cognito-identity-js';
const MEMORY_KEY_PREFIX = '@fcAuth:';
let dataMemory = {};
const syncPromise: Promise<any> = null;
/**
* This is used to set a specific item in storage
*/
function setItem(key: string, value: any) {
AsyncStorage.setItem(MEMORY_KEY_PREFIX + key, value);
dataMemory[key] = value;
return dataMemory[key];
}
/**
* This is used to get a specific key from storage
*/
function getItem(key: string) {
return Object.prototype.hasOwnProperty.call(dataMemory, key)
? dataMemory[key]
: undefined;
}
/**
* This is used to remove an item from storage
*/
function removeItem(key: string) {
AsyncStorage.removeItem(MEMORY_KEY_PREFIX + key);
return delete dataMemory[key];
}
/**
* This is used to clear the storage
*/
function clear() {
dataMemory = {};
return dataMemory;
}
/**
* Will sync the MemoryStorage data from AsyncStorage to storageWindow MemoryStorage
*/
async function sync() {
if (!syncPromise) {
try {
// syncPromise = new Promise((res, rej) => {
const keys = await AsyncStorage.getAllKeys();
const memoryKeys = keys.filter((key: string) =>
key.startsWith(MEMORY_KEY_PREFIX),
);
const stores: string[][] = await AsyncStorage.multiGet(memoryKeys);
stores.map((store: string[]) => {
const key = store[0];
const value = store[1];
const memoryKey = key.replace(MEMORY_KEY_PREFIX, '');
dataMemory[memoryKey] = value;
});
} catch (err) {
console.error(err);
}
}
return syncPromise;
}
export const AuthStorage: ICognitoStorage & {sync: () => Promise<any>} = {
setItem,
getItem,
removeItem,
clear,
sync,
};
Auth.configure({
storage: AuthStorage,
}); |
Any updates on this? |
Hi All, There is a reason we have not yet migrated to the community version of AsyncStorage yet, and that is because of continued support for Expo managed apps. Also while we get deprecation warnings in React Native, AsyncStorage is still available and works. Thus we felt the best way to support both sets of (React Native CLI and Expo) users was to continue using AsyncStorage from React Native at this time. I had previously created a PR back in Nov to migrate to the community edition of AsyncStorage #4402 However we didnt end up releasing that as expo users would need to eject their app, since the community edition of AsyncStorage was not available for expo managed apps at the time. This would be a major issue if all our expo users now had to eject their apps. So we are currently waiting for expo to add this dependency so that we can migrate Amplify JS to use react-native-community/async-storage. Here is the feature request being tracked by Expo: https://expo.canny.io/feature-requests/p/continued-support-for-asyncstorage We are open to suggestions but I want to make sure you all are aware why we are holding off on this. One thing we could do is to create an npm tag specifically for ReactNativeCLI apps with the change, and then we can merge it into master once Expo supports react-native-community/async-storage. Alternatively you can go with a solution like the one by @sofyan-ahmad above |
@Ashish-Nanda Is there a plan to merge this PR? |
It would probably better for the reviewer if the conflicts are solved 😃 |
Any updates on this? Should we create a different pull request and get it merged? |
hi, any updates for this in 2021? 🙄 |
any updates? |
need this to merge asap |
@@ -67,6 +67,7 @@ | |||
"js-cookie": "^2.1.4" | |||
}, | |||
"devDependencies": { | |||
"@react-native-community/async-storage": "^1.6.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jpaas can you resolve this?
@@ -84,7 +85,7 @@ | |||
"eslint-plugin-standard": "^3.0.1", | |||
"jsdoc": "^3.4.0", | |||
"react": "^16.0.0", | |||
"react-native": "^0.44.0", | |||
"react-native": "^0.60.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe update RN version here too
Hi All, |
This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs. Looking for a help forum? We recommend joining the Amplify Community Discord server |
Attempt to fix #3422 by changing AsyncStorage dependency to use @react-native-community/async-storage.
I think a major release bump will be needed in order to separate support for >RN0.60 from <RN0.60.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.