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 ignoredPaths option to ignore serializability check #320

Merged
merged 5 commits into from
Jan 19, 2020

Conversation

kevin940726
Copy link
Contributor

@kevin940726 kevin940726 commented Jan 17, 2020

Fix #319

Add a new option to createSerializableStateInvariantMiddleware called ignoredPaths. It accepts an array of dot-separated path strings to be ignored when checking for serializability in state.

const serializableStateInvariantMiddleware = createSerializableStateInvariantMiddleware(
  {
    ignoredPaths: ["testSlice.a", "testSlice.b.c"]
  }
);

@netlify
Copy link

netlify bot commented Jan 17, 2020

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

Built with commit ee11ed7

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

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 17, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ee11ed7:

Sandbox Source
nostalgic-agnesi-8uepy Configuration
infallible-cohen-kyvfe Configuration
rsk-github-issues-example Configuration

@markerikson
Copy link
Collaborator

Can we try doing ignoredPaths instead?

@kevin940726
Copy link
Contributor Author

@markerikson Sure! But I'm wondering what's the use case for making just part of the slices ignored for checking serializability? From my understanding, it should mainly comes from an uncontrolled third-party library which is also rare. I'm open to both solutions though, I'll make the change later :)

@phryneas
Copy link
Member

createSlice slices do not necessarily have to correspond to a key with their name in the rootReducer, which this seems to be assuming if I read it right - so it introduces a second interpretation of slice that is distinct from createSlice.

@markerikson
Copy link
Collaborator

I'm mostly just looking for a granularity thing here.

@kevin940726 kevin940726 changed the title Add ignoredSlices option to ignore serializability check Add ignoredPaths option to ignore serializability check Jan 17, 2020
@kevin940726
Copy link
Contributor Author

Updated! Thx @markerikson @phryneas ❤️

Do we want to also support passing array of array here? even though dot-separated path is quite common.

const serializableStateInvariantMiddleware = createSerializableStateInvariantMiddleware(
  {
    ignoredPaths: [["testSlice", "a"], ["testSlice", "b", "c"]]
  }
);

@phryneas
Copy link
Member

Updated! Thx @markerikson @phryneas heart

Do we want to also support passing array of array here? even though dot-separated path is quite common.

const serializableStateInvariantMiddleware = createSerializableStateInvariantMiddleware(
  {
    ignoredPaths: [["testSlice", "a"], ["testSlice", "b", "c"]]
  }
);

Hm. I believe once we allow for array notation there, people are gonna assume that we have TS support for that (which I wouldn't want to add here as it's too much complication for a niche use case), so I'd stick with dot notation only.

@markerikson
Copy link
Collaborator

Great, thanks!

@markerikson markerikson merged commit 1b39f25 into reduxjs:master Jan 19, 2020
@kevin940726 kevin940726 deleted the ignored-slices branch January 20, 2020 01:19
@jakeboone02
Copy link
Contributor

Is anyone working on updating the documentation for this? I may have just volunteered...

@markerikson
Copy link
Collaborator

Hmm. Yeah, I missed that the doc update was only for the "other exports" page, and we should mention something about this on the getDefaultMiddleware page too.

@kevin940726
Copy link
Contributor Author

Oops, I didn't know that there is documentation for this in that page. Maybe a link point to the `other exports" page would be fine?

@markerikson
Copy link
Collaborator

Mmm... let's at least update the type signature in the getDefaultMiddleware page, and then update the "API Reference" section to also point to the relevant API options links for redux-immutable-state-invariant and redux-thunk in their repos, and the serializable middleware (the "Other Exports" page).

@kevin940726
Copy link
Contributor Author

kevin940726 commented Jan 22, 2020

Sounds good to me. Currently redux-immutable-state-invariant and redux-thunk already have links pointing to their repos, but are not clear at all. Should we fix that too?

image

Like shown above, redux-immutable-state-invariant is actually clickable but serializable-state-invariant-middleware is not

@markerikson
Copy link
Collaborator

Yeah, I know I made some CSS tweaks to the Redux core repo to make sure links in code snippets are actually colored. Probably ought to pull those over.

@kevin940726
Copy link
Contributor Author

PR opened: #327

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.

Add option to skip serializability check for state slices
4 participants