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

Avoid options mutation #85

Closed

Conversation

betocantu93
Copy link
Contributor

Noticed format and date mutate te options hash, this causes trouble for the new tracking system in octane

@snewcomer
Copy link
Collaborator

This would be good to get in. Need to merge in master though. Significant updates to the date helper though 😞

@betocantu93
Copy link
Contributor Author

betocantu93 commented Feb 8, 2022

hello @snewcomer wondering if this still needed with the last release? so I can merge in master?

🤔 I think it is!

@betocantu93
Copy link
Contributor Author

closed in favor of #119

@snewcomer
Copy link
Collaborator

Yes I think it is but I imagine there is a test you can add!

@betocantu93
Copy link
Contributor Author

I agree a test could be useful, but If I can recall correctly it has something to do with the double render issues in some autotracking situations and so to be honest reproducing a test in this repo could be hard. In my opinion, these functions should not need to know what context they're running in (autotracking, etc) they should be pure stateless functions, which makes me wonder if Object.assign is still not o.k., and that we should be cloning instead

@betocantu93
Copy link
Contributor Author

I can add a test ensuring we don't mutate

@betocantu93 betocantu93 deleted the dont-mutate branch February 9, 2022 04:18
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