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

_createPartialApplicator: avoid passing in wrong count of arguments #2940

Closed

Conversation

adispring
Copy link
Member

@adispring adispring commented Dec 20, 2019

If I have a function created by partialRight, and passing in wrong count of arguments, this will lead to wrong behavior of original function. For example:

const greet = (salutation, title, firstName, lastName) =>
  salutation + ', ' + title + ' ' + firstName + ' ' + lastName + '!';

const greetMsJaneJones = R.partialRight(greet, ['Ms.', 'Jane', 'Jones']);

greetMsJaneJones('Hello', 'Mr.' 'Green'); //=> 'Hello, Mr. Green Jones!'

So function created by partialRight, should be restricted to the right count of parameters?

@adispring adispring force-pushed the optimize-_createPartialApplicator branch from f82c69a to 908ea34 Compare December 20, 2019 03:20
@adispring adispring force-pushed the optimize-_createPartialApplicator branch from 908ea34 to 29d2715 Compare December 20, 2019 03:35
@CrossEye
Copy link
Member

This is definitely a problem. But I don't think it's the right fix. I think the behavior should be something like this:

const foo = (a, b, c, d, ...rest) => ({a, b, c, d, rest})

foo (1, 2, 3, 4, 5, 6)
//=> {a: 1, b: 2, c: 3, d: 4, rest: [5, 6]}

partialRight(foo, [100, 200])(1, 2, 3, 4)
//=> {a: 1, b: 2, c: 100, d: 200, rest: [3, 4]}

Right now that would return {a: 1, b: 2, c: 3, d: 4, rest: [100, 200]}. I'm guessing the easiest way would be to simply abandon that helper, which isn't doing much anyway, and rewrite it in some ES5 version of

const partialRight = (fn, origArgs) => (...args) => 
  fn (...insertAll (fn.length - origArgs.length, origArgs, args))

@CrossEye
Copy link
Member

closing in favor of #2941

@CrossEye CrossEye closed this Jan 23, 2022
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