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

ImmutableStateInvariantMiddleware affecting tests in dev environment (#23) #219

Merged
merged 4 commits into from
Jun 24, 2024

Conversation

sxpydo
Copy link

@sxpydo sxpydo commented Jun 20, 2024

Ticket: https://github.com/isaaccomputerscience/isaac-cs-issues/issues/23

Sometimes when running tests in React app a console warning will appear:

"ImmutableStateInvariantMiddleware took 106ms, which is more than the warning threshold of 32ms.
If your state or actions are very large, you may want to disable the middleware as it might cause too much of a slowdown in development mode."

Copy link

github-actions bot commented Jun 20, 2024

Lines Statements Branches Functions
Coverage: 50%
50.89% (8614/16924) 34% (4177/12285) 40.18% (1449/3606)

@sxpydo sxpydo marked this pull request as ready for review June 20, 2024 18:16
const newMiddleware = getDefaultMiddleware(defaultMiddlewareOptions).concat(middleware);
const newMiddleware = getDefaultMiddleware({
immutableCheck: false,
serializableCheck: false,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this one be false also? I thought we were only concerned about the immutability check as that's the one being noisy?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right! I will make changes to this on Monday morning!

return newMiddleware;
},
preloadedState: {},
devTools: process.env.NODE_ENV !== "production",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not need these?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so because it should keep the preloadedState and devTools configurations as they are important for initialising the Redux store with a predefined state and enabling Redux DevTools during development.

  • middleware: Disables only the immutableCheck while keeping the serializableCheck enabled.
  • preloadedState: Initialises the store with an empty state object - it can be customised if needed.
  • devTools: Enables Redux DevTools only in the development environment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok but you've removed them is what I mean ;)

Copy link
Author

@sxpydo sxpydo Jun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insert facepalm emoji - that was very unintentional

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rectified it now!

Copy link

@sxpydo sxpydo merged commit b277851 into main Jun 24, 2024
4 checks passed
@sxpydo sxpydo deleted the shav/immutable branch June 24, 2024 12:35
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