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

Allow consumer to augment middleware to tolerate certain structures (e.g. Immutable) #141

Merged
merged 4 commits into from
Jul 6, 2019

Conversation

kgregory
Copy link
Contributor

@kgregory kgregory commented May 7, 2019

In Issue #140 I said I would wait before submitting a PR, but here we are.

This pull request will enhance serializable-state-invariant-middleware to accept an additional option at creation: getEntries. Additionally, it will use the isSerializable when checking action and state.

These two options will allow a consumer to augment the middleware to tolerate certain structures, such as ImmutableJS. In the example below, the consumer is configuring the middleware to permit immutable structures and consider them to be serializable:

// immutable version 3.8.2 assumed for this demo!
const isSerializable = (value: any): boolean =>
  Iterable.isIterable(value) || isPlain(value);

const getEntries = (value: any): [string, any][] =>
  Iterable.isIterable(value) ? value.entries() : Object.entries(value);

const middleware = createSerializableStateInvariantMiddleware({
  isSerializable,
  getEntries,
});

- `isSerializable` function should be used for action and state
- Add `getEntries` function to options that can be used to retrieve nested values
- Modify findNonSerializableValue to use `getEntries` or `Object.entries` to find nested values.
@netlify
Copy link

netlify bot commented May 7, 2019

Deploy preview for redux-starter-kit-docs ready!

Built with commit dbe90c6

https://deploy-preview-141--redux-starter-kit-docs.netlify.com

@kgregory
Copy link
Contributor Author

Additional commit to amend docs for new middleware option

@mayteio
Copy link

mayteio commented Jun 12, 2019

Anything holding this back from being merged?

@markerikson
Copy link
Collaborator

@mayteio : me, mostly :) I've been swamped with work, spent the couple weeks prepping for a conference talk, just gave that, and will be doing more traveling the next few days. I haven't had time to sit down and play maintainer for RSK.

Hopefully I'll have some time in the next couple weeks.

@kgregory
Copy link
Contributor Author

@markerikson haven't pushed on this because of exactly those reasons. You do a crazy amount of good work that helps so many developers. Thanks for that!

@markerikson
Copy link
Collaborator

Okay, seems reasonable. Merging.

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.

3 participants