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

Allow .current to return CustomUser #680

Merged
merged 1 commit into from
Nov 4, 2019

Conversation

taivo
Copy link
Contributor

@taivo taivo commented Nov 3, 2019

If we do Parse.Object.registerSubclass('CustomUser', CustomUser);, parse still serializes the current user in local storage with className="_User". As such, in subsequent loading of the app, the deserialization process will know nothing of the CustomUser class and CustomUser.current() will just return a Parse.User. Thus we lose access to all the cool custom methods. Changing the registration to Parse.Object.registerSubclass('_User', CustomUser) fixes that.

If we do `Parse.Object.registerSubclass('CustomUser', CustomUser);`, parse still serializes the current user in local storage with `className="_User"`. As such, in subsequent loading of the app, the deserialization process will know nothing of the CustomUser class and `CustomUser.current()` will just return a `Parse.User`. Thus we lose access to all the cool custom methods. Changing the registration to `Parse.Object.registerSubclass('_User', CustomUser)` fixes that.
@TomWFox TomWFox requested a review from dplewis November 3, 2019 21:24
@acinader acinader merged commit c63cea1 into parse-community:gh-pages Nov 4, 2019
@acinader
Copy link
Contributor

acinader commented Nov 4, 2019

thank you!

@TomWFox TomWFox removed the request for review from dplewis November 4, 2019 18:47
@dplewis
Copy link
Member

dplewis commented Nov 4, 2019

While this does fix your issue, it would introduce another problem.

You would override ParseObject.registerSubclass('_User', ParseUser); here (I don't know what the side effects would be)

I did a PR recently for better support for user subclassing but I missed current.

@acinader
Copy link
Contributor

acinader commented Nov 4, 2019

ugh. I didn't fully understand now that you explain it. revert?

@taivo
Copy link
Contributor Author

taivo commented Nov 4, 2019

I would hold off on the revert. I traced the sdk code for .current() for about an hour and saw that it's littered with the hardcoded _User string and its association with ParseUser. I noticed the line ParseObject.registerSubclass('_User', ParseUser); and overriding that seems like a less risky option than dealing with all the hardcoded cases without a full understanding of the code. As long as the CustomUser class extends the ParseUser class, that makes CustomUser is a ParseUser.

Also, iirc during my trace, there are times when user processing work is delegated to CoreManager and the this context of CustomUser is lost. Also, parse seems to prevent the setting of the className field. That field gets written to _User so the raw data necessary to deserialize into a CustomUser currently isn't there.

I feel this is a good stop gap solution to help people get their own projects moving even though it's not ideal. I recommend we leave this in place until @dplewis or someone else has a full solution implemented.

And for what it's worth, a complete solution should also cover the various login providers.

@dplewis
Copy link
Member

dplewis commented Nov 4, 2019

CustomUser here is simple but a more complex subclass could break your SDK. There is an open issue in JSSDK that shows this.

I’ll look into current, fromJSON should return the subclass. I’ll write a test case

@dplewis
Copy link
Member

dplewis commented Nov 4, 2019

@taivo I just wrote a test case and CustomUser.current() works.

parse-community/Parse-SDK-JS#978

Can you let me know if any changes can replicate your issue?

dplewis added a commit that referenced this pull request Nov 5, 2019
Probably should note that all static user functions will support subclassing after the next js release.
@taivo
Copy link
Contributor Author

taivo commented Nov 5, 2019

@dplewis is there a way to test the effect of reloading a page? From what I can see, the object user: CustomUser is retained in browser memory after signUp/logIn and serialized into local storage. Without a page reload, the memory cache of user: Customer gets prioritized. When you reload a page, that cache is non-existent and deserialization from local storage takes place. That's when you see the problem. The crux of it seems to revolve around the className field being serialized as _User instead of CustomUser.
Not sure if this helps, but I spotted this issue while working with an ionic framework app so my thinking revolves around that context. Essentially every time I made a change, ionic reloads the page and boom, my CustomUser is gone and replaced with ParseUser

dplewis added a commit that referenced this pull request Nov 5, 2019
Probably should note that all static user functions will support subclassing after the next js release.
@dplewis
Copy link
Member

dplewis commented Nov 5, 2019

@taivo Yes there is Parse.User._clearCache(). I'll update the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants