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

Hooks Accounts.onLogin/onLoginFailure should be available on client #3572

Closed
jakozaur opened this issue Jan 28, 2015 · 10 comments
Closed

Hooks Accounts.onLogin/onLoginFailure should be available on client #3572

jakozaur opened this issue Jan 28, 2015 · 10 comments

Comments

@jakozaur
Copy link
Contributor

@jakozaur jakozaur commented Jan 28, 2015

Currently Accounts hooks onLogin and onLoginFailure are available only on server.

I am trying to add logic that would move user to certain page on login and it is hard to implement without the hook.

However, it looks logically that this methods should be isomorphic on client. This is one of the core Meteor design philosophy.

Happy to try to implement myself, but would like to get blessing from MDG. Do I sound reasonable? 😄

The only tricky part is that the attempt object would be a bit different on server vs. client (user and connection object). Though I would note it in documentation, I wouldn't worry about it.

@stubailo
Copy link
Contributor

@stubailo stubailo commented Jan 28, 2015

Thanks for the feature request! You might be interested in reading our feature request guidelines.

@stubailo
Copy link
Contributor

@stubailo stubailo commented Jan 28, 2015

FWIW I've also have a need for this in the past and Tracker.autorun( .. Meteor.userId() .. ) doesn't always cut it.

@jakozaur
Copy link
Contributor Author

@jakozaur jakozaur commented Jan 29, 2015

Thanks @stubailo so much for the reply!

My previous attempts:
0. Tried onLogin on client without reading docs, didn't work.

  1. Call Meteor.login directly, so I can provide callback. Gotcha: Can't use accounts-ui, need to add it to few places.
  2. Tracer.autorun. Gotcha: Hard to recognize user login vs. page reload. The naive version doesn't work, since Meteor.user is not loaded immediately. Though it may be masked if your localhost is fast.
  3. Use server onLogin hook to change user preferences attribute, than check for it on client. Gotcha: A lot of code for simple thing, prone to bugs. Unnecessary latency...

IMHO, this is common pattern in many apps and can be as simple as:

Accounts.onLogin(function () {
  Router.go("dashboard");
});

Hope to submit some code shortly.

@stubailo
Copy link
Contributor

@stubailo stubailo commented Jan 29, 2015

Yeah I would look at a PR for this.

@AnthonyAstige
Copy link

@AnthonyAstige AnthonyAstige commented Feb 24, 2015

meteor add gwendall:auth-client-callbacks made Accounts.onLogin(...) on the client trigger for me from a quick manual test. I'm not sure on it's internal implementation nor have I tested it in depth.

Reference

@jakozaur
Copy link
Contributor Author

@jakozaur jakozaur commented Feb 24, 2015

Thanks for pointing @AnthonyAstige that out.

Unfortunately the gwendall:auth-client-callbacks is buggy implementation:
https://github.com/gwendall/meteor-auth-client-callbacks/blob/master/client/lib.js

You get callback on each page reload, not only when you login. It is approach #2 I described few comments above.

stubailo added a commit that referenced this issue Feb 24, 2015
@stubailo
Copy link
Contributor

@stubailo stubailo commented Feb 24, 2015

Merged the PR! Right now there is no way of knowing how the login is triggered because we decided not to pass through the arguments, but you can reliably get a callback on login.

@stubailo stubailo closed this Feb 24, 2015
@glasser
Copy link
Member

@glasser glasser commented Feb 24, 2015

I'm not sure I buy the reasoning about not including the method name and arguments, but we can always add it later. But removing the error from the failure hook seems pretty extreme to me.

Overall though this is a great addition to Meteor!

@stubailo
Copy link
Contributor

@stubailo stubailo commented Feb 24, 2015

I think a point could be made that it is good to not have the arguments, because any random client-side library could then add a callback and get the new user's password.

mquandalle pushed a commit to mquandalle/meteor-accounts that referenced this issue Mar 31, 2016
mquandalle pushed a commit to mquandalle/meteor-accounts that referenced this issue Mar 31, 2016
mquandalle pushed a commit to mquandalle/meteor-accounts that referenced this issue Mar 31, 2016
mquandalle pushed a commit to mquandalle/meteor-accounts that referenced this issue Mar 31, 2016
mquandalle pushed a commit to mquandalle/meteor-accounts that referenced this issue Mar 31, 2016
mquandalle pushed a commit to mquandalle/meteor-accounts that referenced this issue Mar 31, 2016
mquandalle pushed a commit to mquandalle/meteor-accounts that referenced this issue Mar 31, 2016
mquandalle pushed a commit to mquandalle/meteor-accounts that referenced this issue Mar 31, 2016
mquandalle pushed a commit to mquandalle/meteor-accounts that referenced this issue Mar 31, 2016
mquandalle pushed a commit to mquandalle/meteor-accounts that referenced this issue Mar 31, 2016
mquandalle pushed a commit to mquandalle/meteor-accounts that referenced this issue Mar 31, 2016
mquandalle pushed a commit to mquandalle/meteor-accounts that referenced this issue Mar 31, 2016
tmeasday pushed a commit to meteor/docs that referenced this issue May 9, 2016
@indesignlatam
Copy link

@indesignlatam indesignlatam commented May 18, 2016

Maybe not returning the whole arguments but at least the type, so on the client we can run code onLogin and code onResume.

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

Successfully merging a pull request may close this issue.

None yet
5 participants