allow for circular references by building reducer lazily on first reducer call #1686

phryneas · 2021-11-03T22:01:27Z

This would solve most long-standing problem of circular references between slices by calling the builder callback lazily when the reducer is called first instead of immediately on reducer creation.

phryneas · 2021-11-03T22:04:05Z

packages/toolkit/src/tests/createSlice.test.ts

@@ -191,7 +191,8 @@ describe('createSlice', () => {
                .addCase('increment', (state) => state + 1)
                .addCase('increment', (state) => state + 1),
          })
-        ).toThrowErrorMatchingInlineSnapshot(
+          slice.reducer(undefined, { type: 'unrelated' })


This is the only downside to this I could find. As the builder is now called lazily on first reducer call, createSlice will not immediately toggle these errors.
But they will be triggered immediately on adding this to a store, so I'm pretty sure that would be okay.

phryneas · 2021-11-03T22:05:28Z

Tests are failing because apparently the integration branch tests are just currently failing. Tests for the PR run fine locally.

github-actions · 2021-11-03T22:06:58Z

size-limit report 📦

Path	Size
1. entry point: @reduxjs/toolkit (cjs.production.min.js)	12.36 KB (+0.32% 🔺)
1. entry point: @reduxjs/toolkit (esm.js)	10.34 KB (+0.27% 🔺)
1. entry point: @reduxjs/toolkit/query (cjs.production.min.js)	22.02 KB (+0.16% 🔺)
1. entry point: @reduxjs/toolkit/query (esm.js)	18.32 KB (+0.22% 🔺)
1. entry point: @reduxjs/toolkit/query/react (cjs.production.min.js)	24.14 KB (+0.14% 🔺)
1. entry point: @reduxjs/toolkit/query/react (esm.js)	21 KB (+0.21% 🔺)
2. entry point: @reduxjs/toolkit (without dependencies) (cjs.production.min.js)	5.69 KB (+0.71% 🔺)
2. entry point: @reduxjs/toolkit (without dependencies) (esm.js)	5.65 KB (+0.56% 🔺)
2. entry point: @reduxjs/toolkit/query (without dependencies) (cjs.production.min.js)	9.99 KB (0%)
2. entry point: @reduxjs/toolkit/query (without dependencies) (esm.js)	10.41 KB (0%)
2. entry point: @reduxjs/toolkit/query/react (without dependencies) (cjs.production.min.js)	2.65 KB (0%)
2. entry point: @reduxjs/toolkit/query/react (without dependencies) (esm.js)	2.49 KB (0%)
3. createSlice (esm.js)	5.24 KB (+0.51% 🔺)
3. createEntityAdapter (esm.js)	5.83 KB (0%)
3. configureStore (esm.js)	5.83 KB (0%)
3. createApi (esm.js)	16.57 KB (+0.23% 🔺)
3. createApi (react) (esm.js)	19.27 KB (+0.18% 🔺)
3. fetchBaseQuery (esm.js)	11.13 KB (+0.43% 🔺)
3. setupListeners (esm.js)	9.95 KB (+0.4% 🔺)
3. ApiProvider (esm.js)	18.1 KB (+0.19% 🔺)

codesandbox-ci · 2021-11-03T22:08:20Z

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 addfab6:

Sandbox	Source
Vanilla	Configuration
Vanilla Typescript	Configuration
rsk-github-issues-example	Configuration
@examples-query-react/basic	Configuration
@examples-query-react/advanced	Configuration

markerikson · 2021-11-04T04:27:23Z

@phryneas Per #1687, the build failures appear to be due to the version: "1.7.0-beta.0" string in package.json causing Yarn v2 to fail to install the local RTK package.

Yarn v3 seems to fix that issue, but the docs aren't building clean atm.

As a workaround, just change the version string back to "1.6.2" on this branch and it should build okay.

markerikson · 2021-11-04T04:32:01Z

packages/toolkit/src/createSlice.ts

  if (!name) {
    throw new Error('`name` is a required option for createSlice')
  }
+  const initialState =


✋ Doesn't this interfere with the lazy init logic I just added to createReducer? We should be able to just pass this through.

Since createReducer will be called later than it was before, I wanted to keep the freezing behaviour as soon as possible. It should not really interfere with it, the "work" is just being done twice.

…ucer call

phryneas · 2021-11-04T20:53:49Z

Grr. TS 3.9 does not have ??= yet.

phryneas · 2021-11-04T21:02:16Z

And it builds green. W00p w00p.

markerikson · 2021-11-04T21:03:50Z

🚢 🚢

MatthewWid · 2021-12-07T03:28:08Z

Hi there. Is there a plan to add similar functionality for createReducer itself too?

After refactoring a module in my project from createReducer back to createSlice this update certainly fixed the circular dependency issue (amazing!) but it would be nice if createReducer itself had the same behaviour.

As a very hacky workaround for createReducer I was able to wrap the builder callback code in a setTimeout(() => {}, 0) and it fixed the issue, but I'm sure this could have some unintended downsides @markerikson ?

markerikson · 2021-12-07T03:33:17Z

@MatthewWid That shouldn't really be a concern in most cases. The circular dependency issue only shows up with createSlice because you may have sliceA wanting to listen to an action defined in sliceB and vice-versa. Since createReducer does not define any new action types, that scenario shouldn't happen in relation to the reducer itself. It could happen if there are other variables in the file that are being imported back and forth, but that isn't createReducer's fault at that point.

Do you have an actual example of a createReducer use that is causing a circular dependency?

MatthewWid · 2021-12-07T11:54:37Z

@markerikson The original code is proprietary so I cannot post it exactly, unfortunately, but I have the taken time to create a repository with a reproduction of what I am talking about:

https://github.com/MatthewWid/rtk-createreducer-circular-dependencies

Running instructions are in README.md, I hope it's easy to follow! For reference, the original project follows the project structure from Bulletproof React which is why it is organised the way it is.

See src/index.ts - it simply runs a series of actions and prints the store state after each.

master demonstrates the issue, showing that there is a circular dependency stemming from an action created by createAsyncThunk used in a separate reducer that is undefined and throws:

  builderCallback(builder)
  ^
TypeError: Cannot read property 'fulfilled' of undefined
    at /src/features/users/slices/user.reducer.ts:13:24
    at executeReducerBuilderCallback (/node_modules/.pnpm/@[email protected]/node_modules/@reduxjs/toolkit/src/mapBuilders.ts:194:3)
    at createReducer (/node_modules/.pnpm/@[email protected]/node_modules/@reduxjs/toolkit/src/createReducer.ts:220:9)
    at Object.<anonymous> (/src/features/users/slices/user.reducer.ts:6:42)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Module.m._compile (/node_modules/.pnpm/[email protected]_d1ba71cee4528a43e185cc3d6fa907dc/node_modules/ts-node/src/index.ts:1371:23)
    at Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Object.require.extensions.<computed> [as .ts] (/node_modules/.pnpm/[email protected]_d1ba71cee4528a43e185cc3d6fa907dc/node_modules/ts-node/src/index.ts:1374:12)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
 ELIFECYCLE  Command failed with exit code 1.

This occurs with the createPost action in the user.reducer.ts file.

If you then checkout the slice-fix branch it implements the exact same functionality, but simply refactored to use slices instead (and is on the @next version of RTK). You can see that it works just fine after. Hooray for the update!

Ideally, the first form on the master branch would work by lazy-loading the initialization for createReducer too.

Thanks so much for your responsiveness and work on RTK!

markerikson · 2021-12-07T17:46:37Z

@MatthewWid yeah, the problem here is you've managed to create a real circular dependency based on your own folder structure:

posts.reducer imports deleteUser
users.reducer imports createPost

and this is made worse by the fact that you are using feature/myFeature/index.ts files to re-export everything together, and then having those reducer files import from that other feature's index files.

This can be avoided entirely given the current file structure if you just have the reducers import directly from those separate action files instead, ie, import { createPost } from 'features/posts/post.actions'.

We might be able to update createReducer with a similar implementation what we did in createSlice, but the bigger issue here is really how you've structured your logic and imports in the first place.

MatthewWid · 2021-12-07T23:14:49Z

@markerikson That's understandable. As I say it's structured from Bulletproof React (that I believe is loosely similar to the Redux docs feature-folder recommendations?):

Everything from a feature should be exported from the index.ts file which behaves as the public API of the feature.

Otherwise you're necessarily relying on the internal structure of one feature from another feature so they are not as decoupled as they could be.

Regardless, you're right in that the circular dependency is created by myself here, though the fact that this update seems to fix this exact issue only for createSlice and not createReducer when the former is supposed to be convenient way to write the latter (among other things) seems unexpected to me.

Thanks for considering, anyway!

markerikson · 2021-12-07T23:19:12Z

The Redux docs do say "use feature folders", but personally I've always disliked the idea of "re-export everything from an index.ts file".

Modifying createReducer should be feasible, it's just never been brought up as the cause of a circular dependency problem before that I can think of because typically you aren't generating action types in that same file the way createSlice does.

phryneas · 2021-12-07T23:20:32Z

Tbh, that bulletproof react recommendation might make sense for externally exported stuff, but that would still not mean that within that module you should import it from the index. That's how you get circularities - the index should only be used from the outside.

phryneas requested a review from markerikson November 3, 2021 22:01

phryneas commented Nov 3, 2021

View reviewed changes

markerikson mentioned this pull request Nov 4, 2021

[TESTING ONLY] Figure out CI issues with 1.7 integration branch #1687

Closed

markerikson reviewed Nov 4, 2021

View reviewed changes

allow for circular references by building reducer lazily on first red…

455809f

…ucer call

phryneas force-pushed the pr/lazy-circularity branch from f27d3f4 to 455809f Compare November 4, 2021 20:46

TS 3.9 compat

addfab6

phryneas marked this pull request as ready for review November 4, 2021 21:02

phryneas merged commit c455fc2 into v1.7.0-integration Nov 4, 2021

renovate bot mentioned this pull request Feb 7, 2022

Update all non-major dependencies leinelissen/aeon#296

Merged

1 task

markerikson mentioned this pull request Jan 5, 2023

Add runtime deprecation warning for object case reducer syntax in createSlice.extraReducers and createReducer #2301

Closed

aryaemami59 deleted the pr/lazy-circularity branch January 3, 2025 07:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow for circular references by building reducer lazily on first reducer call #1686

allow for circular references by building reducer lazily on first reducer call #1686

phryneas commented Nov 3, 2021 •

edited

Loading

phryneas Nov 3, 2021

phryneas commented Nov 3, 2021

github-actions bot commented Nov 3, 2021 •

edited

Loading

codesandbox-ci bot commented Nov 3, 2021 •

edited

Loading

markerikson commented Nov 4, 2021

markerikson Nov 4, 2021

phryneas Nov 4, 2021

phryneas commented Nov 4, 2021

phryneas commented Nov 4, 2021

markerikson commented Nov 4, 2021

MatthewWid commented Dec 7, 2021 •

edited

Loading

markerikson commented Dec 7, 2021 •

edited

Loading

MatthewWid commented Dec 7, 2021

markerikson commented Dec 7, 2021

MatthewWid commented Dec 7, 2021

markerikson commented Dec 7, 2021

phryneas commented Dec 7, 2021

allow for circular references by building reducer lazily on first reducer call #1686

allow for circular references by building reducer lazily on first reducer call #1686

Conversation

phryneas commented Nov 3, 2021 • edited Loading

phryneas Nov 3, 2021

Choose a reason for hiding this comment

phryneas commented Nov 3, 2021

github-actions bot commented Nov 3, 2021 • edited Loading

size-limit report 📦

codesandbox-ci bot commented Nov 3, 2021 • edited Loading

markerikson commented Nov 4, 2021

markerikson Nov 4, 2021

Choose a reason for hiding this comment

phryneas Nov 4, 2021

Choose a reason for hiding this comment

phryneas commented Nov 4, 2021

phryneas commented Nov 4, 2021

markerikson commented Nov 4, 2021

MatthewWid commented Dec 7, 2021 • edited Loading

markerikson commented Dec 7, 2021 • edited Loading

MatthewWid commented Dec 7, 2021

markerikson commented Dec 7, 2021

MatthewWid commented Dec 7, 2021

markerikson commented Dec 7, 2021

phryneas commented Dec 7, 2021

phryneas commented Nov 3, 2021 •

edited

Loading

github-actions bot commented Nov 3, 2021 •

edited

Loading

codesandbox-ci bot commented Nov 3, 2021 •

edited

Loading

MatthewWid commented Dec 7, 2021 •

edited

Loading

markerikson commented Dec 7, 2021 •

edited

Loading