Skip to content
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

Closed
wants to merge 1 commit into from
Closed

async storage -> community #4027

wants to merge 1 commit into from

Conversation

jpaas
Copy link

@jpaas jpaas commented Sep 16, 2019

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.

@bneigher
Copy link

🤤

@ejdigby
Copy link

ejdigby commented Oct 6, 2019

Please merge this!

Copy link

@birken92 birken92 left a 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.

Copy link

@alisherakb alisherakb left a 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

@Dellybro
Copy link

When can we expect this to go through? I have many projects that aren't keeping my users credentials because of this.

Copy link

@chk-profiq chk-profiq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix #4272

@TomJKlug
Copy link

Would love to see this merged as well.

@Dellybro
Copy link

Can we get an AWS Rep to review this changes asap?

@sospedra
Copy link

sospedra commented Nov 1, 2019

All these millions on Amazon
And we're here without a single PR merged 😆

Any help AWS pals? 🤷‍♂
I'd be nice to have this merged it's very annoying this YellowWarning box poping in all the time
Plus, this is gonna be deprecated soon 🤔

@idanlo
Copy link

idanlo commented Nov 1, 2019

@sospedra it's already deprecated for people with RN version >=0.60 and the async storage functionality doesn't work for us

@sospedra
Copy link

sospedra commented Nov 2, 2019

Good to know.
Then, this PR shall be close, right?

@markmckim
Copy link

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.

@Dellybro
Copy link

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

import AsyncStorage from '@react-native-community/async-storage';
/*
 * Copyright 2017-2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 *
 * Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with
 * the License. A copy of the License is located at
 *
 *     http://aws.amazon.com/apache2.0/
 *
 * or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
 * CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions
 * and limitations under the License.
 */
const MEMORY_KEY_PREFIX = '@StorageService:';
let dataMemory = {};
/** @class */
export class MemoryStorage {
  /**
   * This is used to set a specific item in storage
   * @param {string} key - the key for the item
   * @param {object} value - the value
   * @returns {string} value that was set
   */
  static setItem(key, value) {
    AsyncStorage.setItem(MEMORY_KEY_PREFIX + key, value);
    dataMemory[key] = value;
    return dataMemory[key];
  }
  /**
   * This is used to get a specific key from storage
   * @param {string} key - the key for the item
   * This is used to clear the storage
   * @returns {string} the data item
   */
  static getItem(key) {
    return Object.prototype.hasOwnProperty.call(dataMemory, key)
      ? dataMemory[key]
      : undefined;
  }
  /**
   * This is used to remove an item from storage
   * @param {string} key - the key being set
   * @returns {string} value - value that was deleted
   */
  static removeItem(key) {
    AsyncStorage.removeItem(MEMORY_KEY_PREFIX + key);
    return delete dataMemory[key];
  }
  /**
   * This is used to clear the storage
   * @returns {string} nothing
   */
  static clear() {
    dataMemory = {};
    return dataMemory;
  }
  /**
   * Will sync the MemoryStorage data from AsyncStorage to storageWindow MemoryStorage
   * @returns {void}
   */
  static sync() {
    if (!MemoryStorage.syncPromise) {
      MemoryStorage.syncPromise = new Promise((res, rej) => {
        AsyncStorage.getAllKeys((errKeys, keys) => {
          if (errKeys) rej(errKeys);
          const memoryKeys = keys.filter(key =>
            key.startsWith(MEMORY_KEY_PREFIX),
          );
          AsyncStorage.multiGet(memoryKeys, (err, stores) => {
            if (err) rej(err);
            stores.map((result, index, store) => {
              const key = store[index][0];
              const value = store[index][1];
              const memoryKey = key.replace(MEMORY_KEY_PREFIX, '');
              dataMemory[memoryKey] = value;
            });
            res();
          });
        });
      });
    }
    return MemoryStorage.syncPromise;
  }
}
MemoryStorage.syncPromise = null;
/** @class */
export default class StorageHelper {
  /**
   * This is used to get a storage object
   * @returns {object} the storage
   */
  constructor() {
    this.storageWindow = MemoryStorage;
  }
  /**
   * This is used to return the storage
   * @returns {object} the storage
   */
  getStorage() {
    return this.storageWindow;
  }
}
import { awsConfig } from './src/aws';
import { MemoryStorage } from './src/modules/MemoryStorage'; // workaround because aws-amplify has a huge bug in it.
awsConfig.Auth['storage'] = MemoryStorage;
Amplify.configure(awsConfig);

@markmckim
Copy link

Is this not being merged because there is a merge conflict? If so, @jpaas can you update your PR?

@tfendt
Copy link

tfendt commented Nov 25, 2019

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",

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

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?

@sofyan-ahmad
Copy link

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,
});

@woltsu
Copy link

woltsu commented May 7, 2020

Any updates on this?

@Ashish-Nanda
Copy link
Contributor

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

@belal-mazlom
Copy link

@Ashish-Nanda
I think expo already support now AsyncStorage community edition!
https://docs.expo.io/versions/latest/sdk/async-storage/

Is there a plan to merge this PR?

@Naturalclar
Copy link

It would probably better for the reviewer if the conflicts are solved 😃

@Cezar-Bytex
Copy link

Any updates on this? Should we create a different pull request and get it merged?

@nguyenthetoan
Copy link

nguyenthetoan commented Feb 3, 2021

hi, any updates for this in 2021? 🙄

@sotomaque
Copy link

sotomaque commented Feb 24, 2021

any updates?

@noumansakhawat-dev
Copy link

need this to merge asap

@@ -67,6 +67,7 @@
"js-cookie": "^2.1.4"
},
"devDependencies": {
"@react-native-community/async-storage": "^1.6.1",

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",

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

@agileurbanite agileurbanite added the React Native React Native related issue label Apr 15, 2021
@Ashish-Nanda
Copy link
Contributor

Ashish-Nanda commented May 10, 2021

Hi All,
The PR #8250 to migrate to the community version of AsyncStorage has just been merged to main. Going to go ahead and close out this PR as a result.
Please refer #6031 (comment) for latest updates on the release.

@github-actions
Copy link

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 *-help channels or Discussions for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
React Native React Native related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AsyncStorage conflict between core and communit