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

Don't require proxies for a correct working #22

Merged
merged 14 commits into from
Jan 3, 2018
Merged

Conversation

mweststrate
Copy link
Collaborator

@mweststrate mweststrate commented Jan 2, 2018

Fixes #8

First implementation is promising and feature complete! Needs some clean up, feature detection etc. Performance is quite neat so far (but bare in mind that the Proxy impl is not yet optimized, while the non-proxy impl is)

The update-10k-todos-in-a-set-of-100K benchmark:

    ✓ just mutate (2ms)
    ✓ deepclone, then mutate (475ms)
    ✓ handcrafted reducer (15ms)
    ✓ immutableJS (65ms)
    ✓ immer - with autofreeze (309ms)    // Immer, with auto freeze enabled
    ✓ immer - without autofreeze (148ms) // Immer, but without auto freeze enabled
    ✓ immer - NO PROXIES - with autofreeze (470ms)
    ✓ immer - NO PROXIES - without autofreeze (428ms)

immer-es5.js Outdated
@@ -0,0 +1,244 @@
import {isObject} from "util"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where isObject is used. Is this a typo?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. auto import :(

@benbraou
Copy link
Collaborator

benbraou commented Jan 3, 2018

Great! 👏
I did not look thoroughly at the implementation details tbh. (I will do it this weekend, but this is independent from this PR of course. For learning purposes 😁)

@mweststrate mweststrate changed the title Don't require proxies for a correct working WIP: Don't require proxies for a correct working Jan 3, 2018
@mweststrate
Copy link
Collaborator Author

@benbraou sorry, forgot to mark it as WIP! Still making some improvements besides the build :)

@mweststrate mweststrate changed the title WIP: Don't require proxies for a correct working Don't require proxies for a correct working Jan 3, 2018
@mweststrate mweststrate merged commit 4165abb into master Jan 3, 2018
@mweststrate mweststrate deleted the feature/es5 branch January 3, 2018 17:15
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.

2 participants