Merge Realm JS and Realm Web types #2950

kraenhansen · 2020-06-09T12:14:11Z

What, How & Why?

The purpose of this PR is to merge duplicated type definitions (such as App, User, UserProfile) across ´types/index.d.ts` and the other files in that directory.

I propose the following changes to the Realm JS types:

User related changes:
- Rename identity on User to id (to align with other SDKs and avoid confusion from authentication provider identities returned when fetching the user profile).
- Rename token to accessToken (to make it more obvious what token this is) and add the refreshToken as a property (this is up for debate - but it makes it easier to test the User implementation).
- Change customData to use a generic type defined by User and the App (like we do for FunctionsFactoryType) to allow users of the API to define types for this custom data, once (when creating the app instance).
- Change the return type of logOut to Promise<void> | void, since it sends a request to invalidate the refresh token.
App related changes:
- Change the allUsers from a method to a getter-property returning a list instead of a UserMap.
- Change the currentUser from a method to a getter-property.
- Have removeUser return a Promise<void> instead of Promise<User>, since that user would have limited use anyway.
Auth related changes:
- Rename from API* to Api.
- Prepend Auth to the class names (Realm.Auth.ApiKeyAuth and Realm.Auth.EmailPasswordAuth).
- A lot of changes to the naming of methods in the provider clients (see commits for details).
- Added a callResetPasswordFunction which was part of the existing Stitch SDK.

I propose the following changest to the Realm Web Types:

App related changes:
- Remove string as a valid argument on methods called with a user parameter (it was ment to enable users to pass in a user ID instead of a user object).
UserProfile related changes:
- Rename pictureURL to pictureUrl.
Auth related changes:
- Rename ApiKeyProvider to ApiKeyAuth.
- Rename EmailPasswordProvider to EmailPasswordAuth.
- Rename get and list to fetch and fetchAll respectively, on the ApiKeyProvider.
- Rename resendConfirmation to resendConfirmationEmail on the EmailPasswordProvider.

These changes lead to some simplifications of the way user state is managed by the app, which also found its way into this PR.

kneth · 2020-06-18T08:34:53Z

packages/realm-web/src/User.ts

+    }
+
+    public async refreshCustomData() {
+        await this.refreshAccessToken();


So in reality, an exception is thrown, right?

Yeah, for now :) Until #2967 gets fixed.

types/app.d.ts

blagoev · 2020-06-18T19:43:52Z

what does this means

Change the return type of logOut to Promise | void, since it sends a request to invalidate the refresh token.

I find it problematic to have the return type differ. Sometimes returning Promise and sometimes void. When would this return void. Or I am not reading this right?

kraenhansen · 2020-06-18T19:51:07Z

I find it problematic to have the return type differ. Sometimes returning Promise and sometimes void. When would this return void. Or I am not reading this right?

Ideally this would just be Promise<void> and you're right, it's confusing.

It's because Realm Web sends a request (like the legacy SDKs) but that doesn't seem to be implemented in the way Object Store logs out a user - or it's implemented in a way that allows it to fail silently.

I propose we change it to make Realm JS return a resolved Promise<void> (even though it's just a synchronous call underneath) and create an issue in object store tracking that it should send the request to invalidate the refresh token. Would that be better?

blagoev · 2020-06-19T10:30:00Z

exactly what I was thinking. if its a synchronous call in some cases this should return a resolved Promise.

steffenagger · 2020-06-18T11:21:01Z

packages/realm-web/src/App.ts

        // Add the user at the top of the stack
-        this.users.splice(0, 0, handle);
+        this.users.splice(0, 0, user);


unshift would just take the user.

steffenagger · 2020-06-23T09:10:58Z

packages/realm-web/src/User.ts

@@ -153,13 +114,7 @@ export class User implements Realm.User {
    private _state: Realm.UserState;
    private transport: AuthenticatedTransport;


A bit inconsistency in prefixing props marked as private with _.
Have we considered to switching away from TS public/private keyword (which doesn't translate ti JS), and adopt JS private class fields.
I'm pretty sure latest versions of TS will emulates the runtime behaviour of private fields the best it can.

The private fields with an _ prefix is only because the class also has a public getter overloading the name. It could be called internalState or something else. But I actually think this specific fields goes away in one of the later PRs.

With regards to JS private fields. It's valuable to keep an eye on, but I don't think the adoption in browsers are good enough for us to rely on. And I don't know the state of polyfils for this?

I think TS handles that, but I'm not sure (as in I haven't tested it) microsoft/TypeScript#30829

Co-authored-by: Kenneth Geisshirt <kenneth.geisshirt@mongodb.com>

kneth · 2020-07-08T11:19:52Z

packages/realm-web/src/App.ts

-            }
+    public switchUser(nextUser: User<FunctionsFactoryType, CustomDataType>) {
+        const index = this.users.findIndex(u => u === nextUser);
+        if (index >= 0) {


index !== -1 might be more precise

kneth · 2020-07-08T11:21:02Z

packages/realm-web/src/App.ts

        // Remove the user from the list of users
-        const index = this.users.findIndex(({ user: u }) => u === user);
+        const index = this.users.findIndex(u => u === user);


Will we always find a user?

If not, findIndex will return -1, which would result in a bug, since the first argument of splice states:

An integer that specifies at what position to add/remove items, Use negative values to specify the position from the end of the array

kneth · 2020-07-08T11:27:18Z

types/app.d.ts

+        logOut(): Promise<void>;
+
+        /**
+         * Link the user with a new identity represented by another set of credentials.


Suggested change

* Link the user with a new identity represented by another set of credentials.

* Link the user with an identity represented by another set of credentials.

kneth · 2020-07-08T11:27:39Z

types/app.d.ts

+        linkCredentials(credentials: Credentials): Promise<void>;
+
+        /**
+         * Call a remote MongoDB Realm function by it's name.


Suggested change

* Call a remote MongoDB Realm function by it's name.

* Call a remote MongoDB Realm function by its name.

* Renamed UserAPIKeyProvider to ApiKeyAuth * Renaming instance methods * Moved from user.auth.apiKeys to user.apiKeys * Renamed js_user_apikey_provider.hpp to js_api_key_auth.hpp * Updated the jsdocs * Adding apiKeys to the browser proxy * Adding a note in the changelog

* Renamed to js_email_password_auth.hpp, email-password-auth-methods.js and api-key-auth-methods.js * Adding a note in the changelog * Updated docs * Renamed to EmailPasswordAuth

* registerEmail -> registerUser * Realm.User.identity -> Realm.User.id. Realm.User.token -> Realm.User.accessToken. Add Realm.User.refreshToken. * Turn Realm.App.{currentUser, allUsers} to an instance property * Add Realm.Auth.EmailPassword.callResetPasswordFunction * Wrongly merged

kraenhansen added the T-Internal label Jun 9, 2020

kraenhansen self-assigned this Jun 9, 2020

kraenhansen changed the title ~~Updating package locks from release~~ Merge Realm JS and Realm Web types Jun 10, 2020

kraenhansen force-pushed the kh/v10-merged-types branch 3 times, most recently from e8b2d7a to a559969 Compare June 10, 2020 23:17

kraenhansen mentioned this pull request Jun 16, 2020

Merge Realm JS and Realm Web TypeScript declarations #2980

Closed

19 tasks

kraenhansen force-pushed the kh/v10-merged-types branch from 3c1205d to 95544ea Compare June 17, 2020 10:55

kraenhansen marked this pull request as ready for review June 17, 2020 10:55

kraenhansen requested review from kneth and steffenagger June 17, 2020 10:55

kneth approved these changes Jun 18, 2020

View reviewed changes

kraenhansen requested a review from blagoev June 19, 2020 12:34

kraenhansen mentioned this pull request Jun 22, 2020

Implement Realm.User.push() #2995

Merged

5 tasks

steffenagger reviewed Jun 23, 2020

View reviewed changes

kraenhansen force-pushed the kh/v10-merged-types branch 2 times, most recently from 5c68a02 to e88cbd5 Compare June 29, 2020 08:11

kraenhansen requested review from steffenagger and kneth July 1, 2020 07:51

kraenhansen force-pushed the kh/v10-merged-types branch 2 times, most recently from 40bd1e7 to b7476c1 Compare July 1, 2020 10:51

Kræn Hansen added 6 commits July 8, 2020 10:43

Updating package locks from release

e99f4fb

Merging types and adjusting configs

9cfbf1c

Fixed linting issues

d1a6271

Merged the auth provider types

d50dfeb

Fixing the functions factory types

6fb981b

Fixed integration tests based on updated types

62337cf

Kræn Hansen and others added 14 commits July 8, 2020 10:44

Merge types and update code accordingly

60777ae

Package-lock got updated

f237a33

Fixed logging out a logged out user

52e20b9

Commented out the App#logOut method

c595141

Adding removeUser to the tests

66f28d9

Update types/app.d.ts

3900e59

Co-authored-by: Kenneth Geisshirt <kenneth.geisshirt@mongodb.com>

Removed "| void"

5e42271

resendConfirmation → resendConfirmationEmail

3d64511

Removed logOut method from the App

211462a

Fixed App constructor type

17faa28

Using unshift instead of splice where applicable

ae2b635

Adopted API to the latest proposal

25698f5

Fixed the realm-network-transport types

9a2919e

Updating integration tests

de28eb4

kraenhansen force-pushed the kh/v10-merged-types branch from b7476c1 to de28eb4 Compare July 8, 2020 08:45

kneth approved these changes Jul 8, 2020

View reviewed changes

kraenhansen and others added 3 commits July 10, 2020 09:49

Merge branch 'v10' into kh/v10-merged-types

615ec13

Merge branch 'v10' into kh/v10-merged-types

a65b534

kraenhansen added the Breaking label Jul 10, 2020

kraenhansen force-pushed the kh/v10-merged-types branch from 349b594 to a65b534 Compare July 10, 2020 10:27

Kræn Hansen and others added 6 commits July 10, 2020 12:36

EmailPasswordAuth renames (#3048)

e267fc6

* Renamed to js_email_password_auth.hpp, email-password-auth-methods.js and api-key-auth-methods.js * Adding a note in the changelog * Updated docs * Renamed to EmailPasswordAuth

Moved push service to a seperate interface

25778b4

Implemented a throwing push for a user

21dcc72

Incorporated feedback

28ccaa8

Removed UserProfile from index.d.ts and updated pictureUrl

4ff57ac

kraenhansen merged commit fb9f1d1 into v10 Jul 10, 2020

kraenhansen deleted the kh/v10-merged-types branch July 10, 2020 13:18

github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge Realm JS and Realm Web types #2950

Merge Realm JS and Realm Web types #2950

kraenhansen commented Jun 9, 2020 •

edited

Loading

kneth Jun 18, 2020

kraenhansen Jun 18, 2020 •

edited

Loading

blagoev commented Jun 18, 2020 •

edited

Loading

kraenhansen commented Jun 18, 2020

blagoev commented Jun 19, 2020

steffenagger Jun 18, 2020

steffenagger Jun 23, 2020

kraenhansen Jun 23, 2020

steffenagger Jun 23, 2020

kraenhansen Jun 23, 2020

kneth Jul 8, 2020

kneth Jul 8, 2020

kraenhansen Jul 10, 2020 •

edited

Loading

kneth Jul 8, 2020

kneth Jul 8, 2020

		@@ -153,13 +114,7 @@ export class User implements Realm.User {
		private _state: Realm.UserState;
		private transport: AuthenticatedTransport;

	* Link the user with a new identity represented by another set of credentials.
	* Link the user with an identity represented by another set of credentials.

	* Call a remote MongoDB Realm function by it's name.
	* Call a remote MongoDB Realm function by its name.

Merge Realm JS and Realm Web types #2950

Merge Realm JS and Realm Web types #2950

Conversation

kraenhansen commented Jun 9, 2020 • edited Loading

What, How & Why?

Choose a reason for hiding this comment

kraenhansen Jun 18, 2020 • edited Loading

Choose a reason for hiding this comment

blagoev commented Jun 18, 2020 • edited Loading

kraenhansen commented Jun 18, 2020

blagoev commented Jun 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraenhansen Jul 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraenhansen commented Jun 9, 2020 •

edited

Loading

kraenhansen Jun 18, 2020 •

edited

Loading

blagoev commented Jun 18, 2020 •

edited

Loading

kraenhansen Jul 10, 2020 •

edited

Loading