Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork866
Update curried-produce.mdx#1070
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Open
Arthur-Milchior wants to merge1 commit intoimmerjs:mainChoose a base branch fromArthur-Milchior:patch-1
base:main
Could not load branches
Branch not found:{{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
Open
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
First, thanks for this library.I hope you'll find this slight rewriting accepatble.On first reading, I foundhttps://immerjs.github.io/immer/curried-produce extremely confusing.The main reason being that, when I read "Passing a function as the first argument", I thought you meant that we were seeing a variation of the `produce` from the previous page, where both arguments were functions. I believe it's important to note that we'll see a very different way to call `produce`.It's even more confusing, because you introduce a new way to call `produce` with an argument, and your example does not use this new way. I spend quite some time trying to understand how this example works. I believe it is better to explain to the user that they are going to see the "bad" version of the example first. Then explain what's wrong with this code (too much boilerplate), and how it can be improved. Finally, once they saw the nice code, explain how it work.I also suspect that, in order to emphasize that `toggleTodo` consists simply of `produce` applied to the function that implements the actual business logic, it's nice to add this business logic as a separate function. This way, we get the very simple `const toggleTodo = produce(toggleTodo_)`. I don't state this would be good production code; I admit that in real life, an anonymous lambda would be better. However, for educational purpose, I find this clearer. This really emphasize that `toggleTodo` is semantically the exact same function as `toggleTodo_` except that it does not uses side effect but output a new value.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First, thanks for this library.
I hope you'll find this slight rewriting accepatble.
On first reading, I foundhttps://immerjs.github.io/immer/curried-produce extremely confusing. The main reason being that, when I read "Passing a function as the first argument", I thought you meant that we were seeing a variation of the
producefrom the previous page, where both arguments were functions. I believe it's important to note that we'll see a very different way to callproduce. It's even more confusing, because you introduce a new way to callproducewith an argument, and your example does not use this new way. I spend quite some time trying to understand how this example works. I believe it is better to explain to the user that they are going to see the "bad" version of the example first. Then explain what's wrong with this code (too much boilerplate), and how it can be improved. Finally, once they saw the nice code, explain how it work.I also suspect that, in order to emphasize that
toggleTodoconsists simply ofproduceapplied to the function that implements the actual business logic, it's nice to add this business logic as a separate function. This way, we get the very simpleconst toggleTodo = produce(toggleTodo_). I don't state this would be good production code; I admit that in real life, an anonymous lambda would be better. However, for educational purpose, I find this clearer. This really emphasize thattoggleTodois semantically the exact same function astoggleTodo_except that it does not uses side effect but output a new value.