[v7-beta] Add test for dynamically injecting reducers #1211
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Deploy preview for react-redux-docs ready! Built with commit 7648588 |
Ephem
commented
Mar 22, 2019
Ephem
commented
Mar 22, 2019
| getByText('Hello world') | ||
| getByText('Hello dynamic world') | ||
| }) | ||
| it('should render child with initial state on the server', () => { |
There was a problem hiding this comment.
Sadly the test-environment can only differ between files, not single tests. If you think it makes more sense, I'll refactor the implementation into a util-file and move this test over to the SSR-suite where the true node test-environment is running, or a new dynamic-reducers-server.spec.js.
|
Can we base this on the v7-beta branch instead? |
Ephem
force-pushed
the
dynamic-reducer-tests
branch
from
1fa133f to
7648588
Compare
|
Done! I suspected you might want that so I had it prepared. 😄 |
Ephem
changed the title
|
Perfect. Thanks! |
markerikson
pushed a commit
that referenced
this pull request
Apr 9, 2019
* Update React to latest * Update React peer dependency to 16.8.x * Initial re-implementation of `connectAdvanced` using hooks Matches changes from v7.0.0-alpha.1 * Update tests to match v7-alpha.1 behavior Added rtl.act() calls around dispatches and other component updates Added clarification on expected mapState calls in some places Disabled some no-longer-relevant tests per implementation Made tests run against React 16.8 by default * adding a react hooks test that fails with v7 alpha * wrapping store.dispatch with rlt.act, fixes component renders * reducing hooks test to 2 components * Fix case where wrapper props changed before store update render * Mark ReactDOM as global in UMD builds Matches state as of v7.0.0-alpha.2 * Fix perf problems with out-of-bounds array access Matches state as of v7.0.0-alpha.3 * Add modules to handle importing batchedUpdates * Use appropriate batched update API for subscriptions * Inject unstable_batchedUpdates in default entry point * Provide an alternate entry point for alternate renderers Matches state as of v7.0.0-alpha.4 * Remove batch arg from createListenerCollection (#1205) This prevents a bug with Terser (webpack's default minifier) where the returned batch function isn't defined due to function inlining. Matches state as of v7.0.0-alpha.5 * Remove older React versions from Travis * Add comments to connectAdvanced. Many much comments! * Re-add test for a custom store as a prop * Fix null pointer exception when store is given as a prop We were trying to read contextValue.subscription, even if that value was null. Reworked logic to handle cases where the store came in as a prop. * Ensure wrapper props are passed correctly when forwarding refs * Add a test to verify subscription passthrough with store-as-prop * add non-batched tests (#1208) * Force SSR tests to mimic a Node environment * Restructure connect tests to group by category for easier reading Yeah, this kills the blame history. Sorry. But it's a lot easier to figure out what the tests are used for now. * Clean up dead code in Provider tests * Add tests to verify errors are thrown for bad mapState functions * Fix edge cases around saved wrapper props and error handling Changed to useLayoutEffect to ensure that the lastWrapperProps ref is written to synchronously when a render is committed. However, because React warns about this in SSR, added a check to fall back to plain useEffect under Node so we don't print warnings. Also added logic to ensure that if an error is thrown during a mapState function, but the component is unmounted before it can render, that the error will still be thrown. This shouldn't happen given our top-down subscriptions, but this will help surface the problem in our tests if we ever break the top-down behavior. * Formatting * Add a test to verify no errors are printed in SSR usage * Ignore .idea/ * 7.0.0-beta.0 * Updated outdated SSR-test (dispatch in ancestors) (#1213) * Added test for injecting dynamic reducers on client and server (#1211) * Remove WebStorm gitignore This goes in a global gitignore file, not a project. * [FIX]: #1219 Save references before update (#1220) * Re-ignore .idea/ * 7.0.0-beta.1 * Update the codecov config to be more forgiving. * add test to verify that mapStateToProps is always called with latest store state (#1215)
albertodev7
pushed a commit
to albertodev7/react-redux
that referenced
this pull request
Dec 8, 2022
* Update React to latest * Update React peer dependency to 16.8.x * Initial re-implementation of `connectAdvanced` using hooks Matches changes from v7.0.0-alpha.1 * Update tests to match v7-alpha.1 behavior Added rtl.act() calls around dispatches and other component updates Added clarification on expected mapState calls in some places Disabled some no-longer-relevant tests per implementation Made tests run against React 16.8 by default * adding a react hooks test that fails with v7 alpha * wrapping store.dispatch with rlt.act, fixes component renders * reducing hooks test to 2 components * Fix case where wrapper props changed before store update render * Mark ReactDOM as global in UMD builds Matches state as of v7.0.0-alpha.2 * Fix perf problems with out-of-bounds array access Matches state as of v7.0.0-alpha.3 * Add modules to handle importing batchedUpdates * Use appropriate batched update API for subscriptions * Inject unstable_batchedUpdates in default entry point * Provide an alternate entry point for alternate renderers Matches state as of v7.0.0-alpha.4 * Remove batch arg from createListenerCollection (reduxjs#1205) This prevents a bug with Terser (webpack's default minifier) where the returned batch function isn't defined due to function inlining. Matches state as of v7.0.0-alpha.5 * Remove older React versions from Travis * Add comments to connectAdvanced. Many much comments! * Re-add test for a custom store as a prop * Fix null pointer exception when store is given as a prop We were trying to read contextValue.subscription, even if that value was null. Reworked logic to handle cases where the store came in as a prop. * Ensure wrapper props are passed correctly when forwarding refs * Add a test to verify subscription passthrough with store-as-prop * add non-batched tests (reduxjs#1208) * Force SSR tests to mimic a Node environment * Restructure connect tests to group by category for easier reading Yeah, this kills the blame history. Sorry. But it's a lot easier to figure out what the tests are used for now. * Clean up dead code in Provider tests * Add tests to verify errors are thrown for bad mapState functions * Fix edge cases around saved wrapper props and error handling Changed to useLayoutEffect to ensure that the lastWrapperProps ref is written to synchronously when a render is committed. However, because React warns about this in SSR, added a check to fall back to plain useEffect under Node so we don't print warnings. Also added logic to ensure that if an error is thrown during a mapState function, but the component is unmounted before it can render, that the error will still be thrown. This shouldn't happen given our top-down subscriptions, but this will help surface the problem in our tests if we ever break the top-down behavior. * Formatting * Add a test to verify no errors are printed in SSR usage * Ignore .idea/ * 7.0.0-beta.0 * Updated outdated SSR-test (dispatch in ancestors) (reduxjs#1213) * Added test for injecting dynamic reducers on client and server (reduxjs#1211) * Remove WebStorm gitignore This goes in a global gitignore file, not a project. * [FIX]: reduxjs#1219 Save references before update (reduxjs#1220) * Re-ignore .idea/ * 7.0.0-beta.1 * Update the codecov config to be more forgiving. * add test to verify that mapStateToProps is always called with latest store state (reduxjs#1215)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As asked for in #1177, this PR contains a new integration test-file
dynamic-reducers.spec.jswith an implementation for dynamically injecting reducers during the render phase with one test for the client and one for the server.Right now it only tests the basic expectation that the wrapped child renders with the initial state from the dynamically injected reducer, this seemed most valuable to me to start with, but can be easily expanded upon. With more tests and tweaks, I guess this implementation could grow/morph to become some form of semi-documented best practice for dynamic reducer injection as long as no public APIs exists.
The reason we do this as a side effect in the render phase is to support SSR, see the comment in the test file for slightly more details.
redux-dynamic-modulesdoes something similar to this, but executes the side effect inconstructinstead, I tried to simplify by placing all logic in render. Instead of relying on the constructor only running once, this has guards to determine when injection and possible patching ofstoreStateneeds to happen or not during rendering.By only patching
storeStateif it exists and otherwise relying on connected child components to fetch an updated state themselves, this test works both withv6and the currentv7-beta.Do note that this is a pretty fragile test that relies on internal and undocumented implementation details of Redux (and probably React), but as @markerikson pointed out, it's hopefully better than nothing. 🤷♂️
I am very open to feedback on how to improve this further, besides from maintainers, I would love feedback from @abettadapur and @mpeyper 😄