Skip to content

React native compatibilize #152

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

Merged
merged 5 commits into from
Oct 22, 2015
Merged

React native compatibilize #152

merged 5 commits into from
Oct 22, 2015

Conversation

leeyeh
Copy link
Contributor

@leeyeh leeyeh commented Sep 21, 2015

see #134.

修复了 xmlhttprequest 模块未找到的问题。
兼容 AsyncStorage

现在有一个问题:
由于 React Native 的 AsyncStorage 是异步的,所以所有直接或间接地调用了 localStorage 模块的 API 都是异步的了,其中原来是同步的、对用户 public 的 API 有:

  • new AV.File(); // 需要关联当前用户为 owner
  • AV.User.current()
  • AV.User.logOut()

一种方案是,保留原有的同步 API,给所有的 API 增加一个同名的异步 API,比如AV.User.currentAsync()。这个方案的问题是用户以及大量内部 API 需要关心当前的运行环境来选择调用哪个 API。
第二种方案是把原有的 API 变为异步(同步也可以包装成异步)。这个的问题是不向前兼容。

还有不管怎样都有的问题是,异步的 new AV.File(); 好别扭。

Fix #158.

@leeyeh
Copy link
Contributor Author

leeyeh commented Sep 21, 2015

cc @Hybrid-Force

@aisk
Copy link
Contributor

aisk commented Sep 21, 2015

Parse 是如何实现的?

@leeyeh
Copy link
Contributor Author

leeyeh commented Sep 21, 2015

Parse 的 File 不依赖 currentUser,logOut() 是异步的,分了 currentUser() 与 currentUserAsync()。
虽然没有试,但是很多 API 在 react native 下调用应该会直接抛 Synchronous storage is not supported by the current storage controller 异常。

@aisk
Copy link
Contributor

aisk commented Sep 21, 2015

现在 AV.User 相关的部分,已经改成返回 promise 的 async 形式了吗?我觉得这个还是需要慎重,毕竟会影响现在所有用户的代码。

我觉得我们可以尝试增加异步 API,然后在文档中建议大家统一使用异步 API。

@Hybrid-Force
Copy link
Contributor

@leeyeh 这版的更改是把原有的AV.User.current API改成异步的了,我觉得这个改动比较激进。一方面,这对现有用户的代码的影响比较大;另一方面,这会增大与Parse的JS SDK的差异。

和Parse的API保持一致,不光使用户的迁移成本降低,而且,对于一些已有的针对Parse的插件,可以比较方便的移植一下配合LeanCloud JS SDK使用。

Parse的1.5.0版本SDK里面,加入了_currentAsync方法,但没有将其作为公共API,而是继续使用Parse.User.current。里面的做法是:

// https://parse.com/downloads/javascript/parse-1.5.0.js
current: function() {
      ...
      if (Parse.Storage.async) {
        // We can't return the current user synchronously
        Parse.User._currentAsync();
        return Parse.User._currentUser;    <-- note that the user might not have been loaded from disk already
      }
      ...

Parse JS SDK的1.6.0版本进行了大规模重构,正式添加Parse.User.currentAsync API,并且Parse.User.current不支持AsyncStorage,期望用户在React Native环境中使用正确的currentAsync API。

SDK中用到localStorage的地方有两个:获取当前用户,和获取installationId。分别会影响到:AV.User.currentAV.User.logoutAV.File构造,HTTP requrest发送(AV._request

针对这几个影响到的地方,我建议:

  • 保留AV.User.current的同步API
    • 并且参照Parse 1.5.0的做法提供不完全的AsyncStorage的支持
    • 可以在SDK初始化的时候提前从storage中加载current user,尽量保证在初次调用AV.User.current的时候current user就已经在内存里了
  • 增加AV.User.currentAsync API
  • File构造函数中继续使用AV.User.current
  • AV._getInstallationId可以继续保持同步返回。
    • 可以参考1.5.0版本的Parse.User.current的做法,异步加载同步返回,这样使得http请求发送部分的代码不需要更改。
    • 虽然我自己试下来发现installationId是null也无所谓,但是,仍然可以在SDK初始化的时候就生成/加载installationId,尽量确保在初次调用的时候installationId已经加载到内存中了。

这样的话,代码的改动不至于太大。对于browser和node环境的用户来说,没有任何API的改变;对于React Native的用户来说,应该期望他们使用AV.User.currentAsync API,但是原来的AV.User.current API仍然可以将就着用一下。

我有一个branch参考Parse的1.5.0版本做了修改。用了一两周下来还没发现什么明显的问题。如果有需要的话,我可以去整理一下发个PR。

@wangxiao
Copy link
Contributor

这个分支是最终版本了么? @leeyeh

@hjiang hjiang added in progress and removed ready labels Oct 10, 2015
@leeyeh
Copy link
Contributor Author

leeyeh commented Oct 10, 2015

是的,请再看一下。

@leeyeh
Copy link
Contributor Author

leeyeh commented Oct 10, 2015

一个已知的问题,JavaScriptCore 不支持 atob,所以现在通过 base64 构造 File 会报错。需要调查一下有没有替代的 API。

var _ = require('underscore');
var Promise = require('../promise');

var Storage = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Storage 后续会设置上什么量,最好能够在此有个提前定义。
比如

var Storage = {
  // async 用来判断本地存储是同步接口还是异步接口。
  // 默认为 false,当在 React Native 中使用时会变为 true,异步接口。
  async: false
};

@wangxiao
Copy link
Contributor

现在原来接口是否兼容?
另外,补充新增接口的基本测试用例。

@leeyeh
Copy link
Contributor Author

leeyeh commented Oct 12, 2015

原来的接口没有变化,但我不能确定是否引入问题因为现在的测试是跑不通的 😭 ,增加了 currentAsync 接口,我补下测试。

@aisk
Copy link
Contributor

aisk commented Oct 12, 2015

我记得原来是十四个单元测试跑不同。可以尝试下切到 master 分之跑一下。

@@ -78,7 +78,15 @@ _.extend(Promise, /** @lends AV.Promise */ {
*/
as: function() {
var promise = new Promise();
promise.resolve.apply(promise, arguments);
if (arguments[0] && typeof arguments[0].then === 'function') {
Copy link
Contributor

Choose a reason for hiding this comment

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

推荐统一使用 underscore 来处理类型判断。

@leeyeh leeyeh changed the title [WIP] React native compatibilize React native compatibilize Oct 17, 2015
@leeyeh
Copy link
Contributor Author

leeyeh commented Oct 17, 2015

我们是不是可以先发个 RC1 @killme2008

@killme2008
Copy link
Contributor

@leeyeh 可以的,我是想测试下在云引擎下的兼容性,这是 js sdk 最大的用户。

# Conflicts:
#	lib/browserify-wrapper/localstorage-browser.js
@leeyeh leeyeh force-pushed the react-native branch 2 times, most recently from 744eda3 to c1d54b3 Compare October 21, 2015 14:12
@leeyeh
Copy link
Contributor Author

leeyeh commented Oct 21, 2015

Fix #158.

@aisk
Copy link
Contributor

aisk commented Oct 21, 2015

👍

@leeyeh leeyeh force-pushed the react-native branch 3 times, most recently from e1c92d9 to 3d4ab34 Compare October 21, 2015 17:32
@wangxiao
Copy link
Contributor

看了下,没有问题。

wangxiao added a commit that referenced this pull request Oct 22, 2015
@wangxiao wangxiao merged commit 4fd9903 into leancloud:master Oct 22, 2015
@leeyeh leeyeh mentioned this pull request Oct 22, 2015
4 tasks
@leeyeh leeyeh deleted the react-native branch October 27, 2015 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants