computedFn doesn't allow varying number of arguments

[Venryx]

The computedFn function doesn't allow a varying number of arguments, between calls to the function it returns.

This is apparently already known, since the function description says:

computedFn(fn) returns a function with the very same signature. There is no limit on the amount of arguments that is accepted. However, the amount of arguments must be consistent and default arguments are not supported.

Is there a fundamental reason that you can't provide a varying number of arguments? Or has the code just not been made flexible enough yet to allow varying argument counts?

If it's the former (impossible to implement), what's the explanation for that limitation?

If it's the latter (possible to implement), then I plan to do the work of implementing it, as it's pretty important for my current use-case.

Anyway, I'm guessing it is possible, so I'm starting work on trying to implement that functionality; if it's not possible though, please let me know so I can save some time. ^_^

---

[potential question] Why do you need to provide a varying number of arguments?

Basically: Because that's the only way I'm able to send in an array in a way that's compatible with caching.

In more detail: I "unwrap" the array's items, and send each one in as a separate "argument" to the function. This lets the data be cached by the DeepMapEntry chain. When the function actually runs, it then reconstructs the array argument from the constituent items.

[Venryx]

Good news: I successfully modified the DeepMap class to support the args array changing length, which enables the computedFn function to accept a varying number of arguments between calls.

You can see the changes I made to achieve this here (it's only two new lines!): https://github.com/mobxjs/mobx-utils/compare/master...Venryx:Issue232

I haven't made a pull request yet, since there are a few clerical tasks to be done:

1. Add unit tests.
2. Update the documentation.
3. Possibly change the name of the Symbol, or initialize it in a different way. (I remember mobx had a way to fallback to strings if the browser didn't support Symbol, but I forget the specific code used.)

If I were to complete the cleanup tasks above, would you consider merging a pull-request for this feature/enhancement?

[mweststrate]

I think this limitation is there indeed for simplicity. (Since variadiac length can typically be solved in userland by using default arguments or a wrapper function I'd guess). Could you elaborate a bit on how your solution works? Especially, how you avoid the symbol ending up in the original method as argument? Could you demonstrate the correctness of your approach by introducing unit tests?

I'd expect the solution to be more complicated, as with this change the deep map would need to be able to store both a result and children at any node, whereas currently it is either a result or children. So in unit tests I'd like to see demonstrated that you can now make sure that an entry like [a,b,c], 1 doesn't override a cached result for [a, b] etc.

On Mon, Dec 16, 2019 at 2:30 AM Stephen Wicklund [email protected] wrote:

Good news: I successfully modified the DeepMap class to support the args array changing length, which enables the computedFn function to accept a varying number of arguments between calls.

You can see the changes I made to achieve this here (it's only two new lines!): master...Venryx:master https://github.com/mobxjs/mobx-utils/compare/master...Venryx:master

I haven't made a pull request yet, since there are a few clerical tasks to be done:

1. Add unit tests.
2. Update the documentation.
3. Possibly change the name of the Symbol, or initialize it in a different way. (I remember mobx had a way to fallback to strings if the browser didn't support Symbol, but I forget the specific code used.)

If I were to complete the cleanup tasks above, would you consider merging a pull-request for this feature/enhancement?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx-utils/issues/232?email_source=notifications&email_token=AAN4NBA5TIY7YTBK43WBBYTQY3R43A5CNFSM4J3CZ6EKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG5KLXA#issuecomment-565880284, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBGAFV7GYFG7HSAWJEDQY3R43ANCNFSM4J3CZ6EA .

[dev-m1-macbook]

would it be ok to add a warning sign on computeFn docs?

this actually caught me off guard this week 😭

I had a function that were used in 2 ways:

1. someArray.map(someComputeFn)

2. <p> {someComputeFn(a)} </p>

(for some reason, the stacktrace was pointing to close-but-not-exact part of the code 😵‍💫 )