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

Simplified redux setup #6534

Merged
merged 2 commits into from
Feb 21, 2024
Merged

Simplified redux setup #6534

merged 2 commits into from
Feb 21, 2024

Conversation

peterhudec
Copy link
Contributor

@peterhudec peterhudec commented Feb 19, 2024

Description of change

Simplifies Redux setup by:

  • Removing the src/client/middleware.js module, because only one of the three things it exported was actually a middleware.
  • Removed the history, store and sagaMiddleware props from DataHubProvider, because they only ever were passed the same values
  • Generalized the way initial data gets into Redux state from the Nunjucks react-slot macro and how the useless "Unexpected key ..." error (thrown by Redux's combineReducers) is suppressed

Along the way I realized that the reasons why we had the spa-base-path Express middleware are no longer valid and the middleware can be removed, so I removed it alongside the doc that was explaining its usage.

Test instructions

There should be no noticable difference. All existing tests should still be passing.

@peterhudec peterhudec requested a review from a team as a code owner February 19, 2024 15:40
Copy link

cypress bot commented Feb 19, 2024

Passing run #51225 ↗︎

0 70 7 0 Flakiness 0

Details:

Removed the spa-base-path middleware and related docs
Project: data-hub-frontend Commit: 2042cf8b3b
Status: Passed Duration: 08:50 💡
Started: Feb 21, 2024 10:05 AM Ended: Feb 21, 2024 10:14 AM

Review all test suite changes for PR #6534 ↗︎

@peterhudec peterhudec force-pushed the fix/simplify-redux-configuration branch 2 times, most recently from 61c0941 to d13d7d5 Compare February 20, 2024 18:03
@peterhudec peterhudec force-pushed the fix/simplify-redux-configuration branch from d13d7d5 to 2042cf8 Compare February 21, 2024 09:40
Copy link
Contributor

@paulgain paulgain left a comment

Choose a reason for hiding this comment

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

Good to see the back of src/middleware/spa-base-path.js.

@peterhudec peterhudec merged commit 3d035eb into main Feb 21, 2024
16 checks passed
@peterhudec peterhudec deleted the fix/simplify-redux-configuration branch February 21, 2024 11:12
chopkinsmade pushed a commit that referenced this pull request Feb 21, 2024
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