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

Add a function to only freeze (if enabled) #16

Closed
geirsagberg opened this issue Aug 11, 2017 · 6 comments
Closed

Add a function to only freeze (if enabled) #16

geirsagberg opened this issue Aug 11, 2017 · 6 comments

Comments

@geirsagberg
Copy link

When using seamless-immutable, I usually do Immutable.from(result) for all my AJAX-calls, so I am sure it will not be changed further down the line.

It would be nice to have a similar function for ImmutableAssign, that would Object.Freeze if enabled, otherwise it would do nothing.

Example:

const immutableResult = iassign(result)

@JiriZidek
Copy link

Feel free to use DeepFreeze instead...

@geirsagberg
Copy link
Author

geirsagberg commented Aug 11, 2017

I did something like this:

import iassign from 'immutable-assign'
import DeepFreeze from 'deep-freeze'

export const deepFreeze = obj => iassign.freeze ? DeepFreeze(obj) : obj

Would you be opposed to a PR with such a wrapper included in ImmutableAssign?

@JiriZidek
Copy link

Why create dependency on DeepFreeze, when benefit (freezing is usefull for developer only, in production it is no-no) is so small ? Moreover you can add such trival wrapper to your project as single function without forcing all consumers of iassign to bloat their code with DeepFreeze?

@JiriZidek
Copy link

Now I see that DeepFreeze is in current version required anyway...

@engineforce
Copy link
Owner

Hi geirsagberg, I am not a fan of re-exporting of another module, which is antipattern. The better way is that you create your own module (called immutable-util for example), which export iassign() and deepFreeze(). And make your application codes depending on the new module, so you can swap out iassign() in the future if needed.

@geirsagberg
Copy link
Author

geirsagberg commented Aug 12, 2017 via email

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

No branches or pull requests

3 participants