PeterStaev / nativescript-azure-mobile-apps

:cloud: NativeScript plugin for working with Microsoft Azure Mobile Apps services
Apache License 2.0
30 stars 10 forks source link

Social Login #5

Closed georgeedwards closed 8 years ago

georgeedwards commented 8 years ago

As noted in my Slack Message, the android implementation is working and has been tested with Google and Facebook login. I am trying to get access to the functions on the returned user object, so I need to define the User object, I have created a user.d.ts file in the client directory, which I am trying to reference in client.d.ts, but this is throwing compiler errors as that module is not found. What am I doing wrong?

PeterStaev commented 8 years ago

Hi @georgeedwards, have couple of comments:

  1. Please do not add the sample to the main plugin project. It just create an overhead and confusion, so your changes to the vscode file will have to be removed.
  2. MicrosoftAzureMobile.d.ts should not be edited. This the metadata export for the iOS runtime so no manual changes should go there so it can be regenerated.
  3. You have incorrectly changed the constructor parameters in client.d.ts for the MobileServiceClient. Its constructor does not accept provider, but portal URL.

As for the user - in order to keep files separated I would like to separate this as a separate sub module (like i separated the table and the query). So in order to separate this you must create a new folder with a pakcage.json and add user.d.ts, user.android.ts and user.ios.ts files there.

georgeedwards commented 8 years ago

OK, I'll try to refactor the user functionality out now

georgeedwards commented 8 years ago

Hi @PeterStaev , Here is a commit which refactors all the user stuff so far into a separate stub (although I still need the login method on client, much like the getTable method). However, at the moment the module I have created to define a User object isn't being found, e.g.

node_modules/nativescript-azure-mobile-apps/client/client.d.ts(19,39): error TS2307: Cannot find module 'nativescript-azure-mobile-apps/user'.

What have I done wrong with that?

PeterStaev commented 8 years ago

See my comments above. Also seems you are missing a package.json file for the new module

PeterStaev commented 8 years ago

Also you should add a reference to the user/user.d.ts file in azure-mobile-apps.d.ts

georgeedwards commented 8 years ago

How is that?

If you are happy, I'll take a look at the ios implementation and the token caching.

PeterStaev commented 8 years ago

Hey @georgeedwards , changes looks much better now. Thanks! But still 2 comments remain above ;)

As for caching - I was thinking of utilizing {N}'s application-settings module and storing the user id and token there, as it is per application and outside apps do not have access to it. So for example nativescript-azure-mobile-apss.user.userId will be the key for the user id and nativescript-azure-mobile-apss.user.token for the token, so that we are sure we do not mess up with other data stored by the application. What do you think?

georgeedwards commented 8 years ago

Hmm, I thought is might be nicer to implement the function as getUser() to return the current user from the client if one is already logged in, otherwise start the login flow. I have had a play with caching tokens etc. but I can't find a nice way to make this work... Ideally, the application setting would be set on completion of the login promise, which is currently written in the sample app. Maybe, this is something we just demo for in the docs? What do you think @PeterStaev ? I'll do the ios stuff now

PeterStaev commented 8 years ago

Hey @georgeedwards , I think it is best to keep things as close to the native APIs as possible. So there the Client has a loginmethod that on complete returns the User. I think this is how it should work here as well. Sames goes for properties - both android and iOS have properties (readonly) to get the userId and token.

georgeedwards commented 8 years ago

@PeterStaev OK, I have created some setCache and getCache helper functions. However, in setCache, I retrieve the token and userID from the application-settings and then try to initialise the user object, see here. However, that line is throwing an error:

TypeError: user_1.MobileServiceUser is not a function File: "/data/data/inc.tangra.azuremobileservicessample/files/app/tns_modules/nativescript-azure-mobile-apps/user/user.js, line: 17, column: 15

I think that is because I am trying to reference the module from inside the module? That logic naturally feels like it should be kept inside that module, or in a new cache module? Alternatively, it could go into Client or a utils module? What would you prefer? (if that is indeed the issue)

PeterStaev commented 8 years ago

@georgeedwards the problem is that you have not implemented the MobileServiceUserclass. You have only defined it in definitions but d.ts are only for autocomplete/error checking. They are not transpiled to JS at all. So you must implement the MobileServiceUser

georgeedwards commented 8 years ago

@PeterStaev adding a common implimentation in user-common still has the same issue... see my new code here

PeterStaev commented 8 years ago

Hey @georgeedwards , I've added your changes and cleaned them up a bit. Thanks for your contribution!