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

Fix issue #4862. Don't wrap login callbacks with bindEnvironment. #4867

Closed
wants to merge 1 commit into from

Conversation

brettle
Copy link
Contributor

@brettle brettle commented Aug 3, 2015

Changes to packages/callback-hook:

  • Add bindEnvironment option to Hook constructor (defaults to true).
  • Add internal helper function dontBindEnvironment() which does all the
    error handling stuff like Meteor.bindEnvironment() but doesn't change
    the environment.
  • Use dontBindEnvironment() instead of Meteor.bindEnvironment() when
    bindEnvironment option is false.
  • Add tests.

Changes to packages/accounts-base:

  • Create hooks with { bindEnvironment: false }
  • Add test for whether Meteor.userId() is available in login callbacks.

Changes to packages/callback-hook:
- Add bindEnvironment option to Hook constructor (defaults to true).
- Add internal helper function dontBindEnvironment() which does all the
  error handling stuff like Meteor.bindEnvironment() but doesn't change
  the environment.
- Use dontBindEnvironment() instead of Meteor.bindEnvironment() when
  bindEnvironment option is false.
- Add tests.

Changes to packages/accounts-base:
- Create hooks with { bindEnvironment: false }
- Add test for whether Meteor.userId() is available in login callbacks.
@stubailo
Copy link
Contributor

stubailo commented Aug 3, 2015

@brettle makes sense to me. The context of the login callback should be the login attempt, not the code where the callback was attached.

@stubailo
Copy link
Contributor

stubailo commented Aug 3, 2015

Merged here: 587e02a

@stubailo stubailo closed this Aug 3, 2015
@stubailo
Copy link
Contributor

stubailo commented Aug 3, 2015

@brettle thanks for all of the tests!

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

Successfully merging this pull request may close these issues.

None yet

2 participants