Meteor.loginWithPassword should have a similar signature as Accounts.createUser

It feels kind of weird that Accounts.createUser and Meteor.loginWithPassword have different signatures. I propose that Meteor.loginWithPassword is updated so that you can do this:

Meteor.loginWithPassword({ email, password }, (error, result) => {...})

I suppose it should have backwards compatibility so people aren't forced to rewrite their existing code.

Thoughts?

I like your proposal. Feels clear to expect similar and consistent interfaces for functions that are alike. I particularly prefer the approach of passing parameters as objects, which enhances versatility and facilitates quick refactoring if needed.

Hi @jamauro

This is my first open source, can you please guide me?

I see that updating the function definition of loginWithPassword in https://github.com/meteor/meteor/blob/devel/packages/meteor/meteor.d.ts with an object would suffice like below

function loginWithPassword(
user: { username?: string; email?: string; id?: string } | string,
password: string,
callback?: (error?: global_Error | Meteor.Error | Meteor.TypedError, result?: any) => void
): void;

I think it should not break any existing functionality, is this the expected fix?

I suppose it should have backwards compatibility so people aren't forced to rewrite their existing code.

@nachocodoner any thoughts on whether it should be a breaking change or not? It would certainly be cleaner from a core code standpoint to make a breaking change but would require developers to make a small change in their code.

Also I think we'll want Meteor.passwordlessLoginWithTokenAnd2faCode to be updated similarly to Meteor.loginWithPassword as part of this issue.

Hey @abhinavtps, thanks for your willingness to jump on this one. Before diving into the solution, let's wait and see if the Meteor team thinks it's worth introducing a breaking change or not. The code will look quite a bit different depending on the answer to that question.

After we have a clearer picture of what's needed, the bulk of the work will need to happen here:

meteor/packages/accounts-password/password_client.js

Line 56 in c0dd2ab

Meteor.loginWithPassword = (selector, password, callback) => {

and here:

meteor/packages/accounts-passwordless/passwordless_client.js

Line 85 in 849ff8a

    
           Meteor.passwordlessLoginWithTokenAnd2faCode = (selector, token, code, callback) => {

The js doc and .d.ts for these functions will need to be updated as well.

It will also need tests added / updated here: https://github.com/meteor/meteor/blob/devel/packages/accounts-password/password_tests.js to ensure the new proposed way works as expected.

any thoughts on whether it should be a breaking change or not? It would certainly be cleaner from a core code standpoint to make a breaking change but would require developers to make a small change in their code.

I propose avoiding breaking changes by implementing it with polymorphism, supporting both parameter and object approaches. We'll provide a deprecation log for the parameter approach, warning users about the convention change, to be removed in a future release. What are your thoughts?

I like that approach.

@abhinavtps see Nacho’s comment above for the desired solution. If you have follow up questions, feel free to use this thread. 🙂