Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Handle Date and RegExp Objects #112

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ctwoolsey
Copy link

No description provided.

…Exp. If they are, they will not allow the code to treat them as an object and create a proxy of ObjectTreeNode. This allows the calling code to retreive a value as expected instead of a proxy with no value.
@pzuraq
Copy link
Collaborator

pzuraq commented Dec 1, 2021

I'm no longer actively maintaining this library and unfortunately do not have the bandwidth to review/merge this change. Happy to pass permissions/ownership along to someone who can look after it.

@ctwoolsey
Copy link
Author

ctwoolsey commented Dec 1, 2021 via email

@ctwoolsey
Copy link
Author

ctwoolsey commented Dec 4, 2021 via email

@simonihmig
Copy link
Owner

@ctwoolsey I already have permissions on this repo, and could handle your PR. However I have some concerns here.

First, it seems to be a bad practice to store mutable instances in the store. I am not a redux pro, but this is what their own docs say:

Redux actions and state should only contain plain JS values like objects, arrays, and primitives. Don't put class instances, functions, or other non-serializable values into Redux!.

Since we can't just put a Date class instance into the Redux store, we'll track the post.date value as a timestamp string:

Second, even when that was acceptable, I would ask you to provide some tests for your changes.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants