Skip to content

Commit 642be78

Browse files
authored
warn on duplicate reducerPath (#1252)
1 parent 0249817 commit 642be78

File tree

6 files changed

+161
-40
lines changed

6 files changed

+161
-40
lines changed

packages/toolkit/src/query/apiTypes.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export type Module<Name extends ModuleName> = {
4444
}
4545

4646
export interface ApiContext<Definitions extends EndpointDefinitions> {
47+
apiUid: string
4748
endpointDefinitions: Definitions
4849
batch(cb: () => void): void
4950
}

packages/toolkit/src/query/core/apiState.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ export type ConfigState<ReducerPath> = RefetchConfigOptions & {
241241
reducerPath: ReducerPath
242242
online: boolean
243243
focused: boolean
244-
middlewareRegistered: boolean
244+
middlewareRegistered: boolean | 'conflict'
245245
} & ModifiableConfigState
246246

247247
export type ModifiableConfigState = {

packages/toolkit/src/query/core/buildMiddleware/devMiddleware.ts

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,44 @@
11
import type { SubMiddlewareBuilder } from './types'
22

3-
export const build: SubMiddlewareBuilder = ({ api }) => {
3+
export const build: SubMiddlewareBuilder = ({
4+
api,
5+
context: { apiUid },
6+
reducerPath,
7+
}) => {
48
return (mwApi) => {
59
let initialized = false
610
return (next) => (action) => {
711
if (!initialized) {
812
initialized = true
913
// dispatch before any other action
10-
mwApi.dispatch(api.internalActions.middlewareRegistered())
14+
mwApi.dispatch(api.internalActions.middlewareRegistered(apiUid))
1115
}
1216

1317
const result = next(action)
1418

1519
if (api.util.resetApiState.match(action)) {
1620
// dispatch after api reset
17-
mwApi.dispatch(api.internalActions.middlewareRegistered())
21+
mwApi.dispatch(api.internalActions.middlewareRegistered(apiUid))
22+
}
23+
24+
if (
25+
typeof process !== 'undefined' &&
26+
process.env.NODE_ENV === 'development'
27+
) {
28+
if (
29+
api.internalActions.middlewareRegistered.match(action) &&
30+
action.payload === apiUid &&
31+
mwApi.getState()[reducerPath]?.config?.middlewareRegistered ===
32+
'conflict'
33+
) {
34+
console.warn(`There is a mismatch between slice and middleware for the reducerPath "${reducerPath}".
35+
You can only have one api per reducer path, this will lead to crashes in various situations!${
36+
reducerPath === 'api'
37+
? `
38+
If you have multiple apis, you *have* to specify the reducerPath option when using createApi!`
39+
: ''
40+
}`)
41+
}
1842
}
1943

2044
return result

packages/toolkit/src/query/core/buildSlice.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ export function buildSlice({
7878
reducerPath,
7979
queryThunk,
8080
mutationThunk,
81-
context: { endpointDefinitions: definitions },
81+
context: { endpointDefinitions: definitions, apiUid },
8282
assertTagType,
8383
config,
8484
}: {
@@ -258,9 +258,8 @@ export function buildSlice({
258258
const subscribedQueries = ((draft[type] ??= {})[
259259
id || '__internal_without_id'
260260
] ??= [])
261-
const alreadySubscribed = subscribedQueries.includes(
262-
queryCacheKey
263-
)
261+
const alreadySubscribed =
262+
subscribedQueries.includes(queryCacheKey)
264263
if (!alreadySubscribed) {
265264
subscribedQueries.push(queryCacheKey)
266265
}
@@ -339,8 +338,11 @@ export function buildSlice({
339338
...config,
340339
} as ConfigState<string>,
341340
reducers: {
342-
middlewareRegistered(state) {
343-
state.middlewareRegistered = true
341+
middlewareRegistered(state, { payload }: PayloadAction<string>) {
342+
state.middlewareRegistered =
343+
state.middlewareRegistered === 'conflict' || apiUid !== payload
344+
? 'conflict'
345+
: true
344346
},
345347
},
346348
extraReducers: (builder) => {

packages/toolkit/src/query/createApi.ts

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,13 @@
11
import type { Api, ApiContext, Module, ModuleName } from './apiTypes'
22
import type { BaseQueryArg, BaseQueryFn } from './baseQueryTypes'
3-
import type {
4-
SerializeQueryArgs} from './defaultSerializeQueryArgs';
5-
import {
6-
defaultSerializeQueryArgs
7-
} from './defaultSerializeQueryArgs'
3+
import type { SerializeQueryArgs } from './defaultSerializeQueryArgs'
4+
import { defaultSerializeQueryArgs } from './defaultSerializeQueryArgs'
85
import type {
96
EndpointBuilder,
10-
EndpointDefinitions} from './endpointDefinitions';
11-
import {
12-
DefinitionType
7+
EndpointDefinitions,
138
} from './endpointDefinitions'
9+
import { DefinitionType } from './endpointDefinitions'
10+
import { nanoid } from '@reduxjs/toolkit'
1411

1512
export interface CreateApiOptions<
1613
BaseQuery extends BaseQueryFn,
@@ -100,10 +97,10 @@ export interface CreateApiOptions<
10097
): Definitions
10198
/**
10299
* Defaults to `60` _(this value is in seconds)_. This is how long RTK Query will keep your data cached for **after** the last component unsubscribes. For example, if you query an endpoint, then unmount the component, then mount another component that makes the same request within the given time frame, the most recent value will be served from the cache.
103-
*
100+
*
104101
* ```ts
105102
* // codeblock-meta title="keepUnusedDataFor example"
106-
*
103+
*
107104
* import { createApi, fetchBaseQuery } from '@reduxjs/toolkit/query/react'
108105
* interface Post {
109106
* id: number
@@ -206,6 +203,7 @@ export function buildCreateApi<Modules extends [Module<any>, ...Module<any>[]]>(
206203
// placeholder "batch" method to be overridden by plugins, for example with React.unstable_batchedUpdate
207204
fn()
208205
},
206+
apiUid: nanoid(),
209207
}
210208

211209
const api = {

packages/toolkit/src/query/tests/devWarnings.test.tsx

Lines changed: 116 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,28 +20,36 @@ afterEach(() => {
2020
restore()
2121
})
2222

23+
const baseUrl = 'http://example.com'
24+
2325
function createApis() {
2426
const api1 = createApi({
25-
reducerPath: 'api1',
26-
baseQuery: fetchBaseQuery({}),
27+
baseQuery: fetchBaseQuery({ baseUrl }),
28+
endpoints: (builder) => ({
29+
q1: builder.query({ query: () => '/success' }),
30+
}),
31+
})
32+
33+
const api1_2 = createApi({
34+
baseQuery: fetchBaseQuery({baseUrl}),
2735
endpoints: (builder) => ({
2836
q1: builder.query({ query: () => '/success' }),
2937
}),
3038
})
3139

3240
const api2 = createApi({
3341
reducerPath: 'api2',
34-
baseQuery: fetchBaseQuery({}),
42+
baseQuery: fetchBaseQuery({baseUrl}),
3543
endpoints: (builder) => ({
3644
q1: builder.query({ query: () => '/success' }),
3745
}),
3846
})
39-
return [api1, api2] as const
47+
return [api1, api1_2, api2] as const
4048
}
4149

42-
let [api1, api2] = createApis()
50+
let [api1, api1_2, api2] = createApis()
4351
beforeEach(() => {
44-
;[api1, api2] = createApis()
52+
;[api1, api1_2, api2] = createApis()
4553
})
4654

4755
describe('missing middleware', () => {
@@ -50,42 +58,49 @@ describe('missing middleware', () => {
5058
['production', false],
5159
])('%s warns if middleware is missing: %s', ([env, shouldWarn]) => {
5260
;(process.env as any).NODE_ENV = env
53-
const store = configureStore({ reducer: { api1: api1.reducer } })
61+
const store = configureStore({
62+
reducer: { [api1.reducerPath]: api1.reducer },
63+
})
5464
store.dispatch(api1.endpoints.q1.initiate(undefined))
5565
expect(getLog().log).toBe(
5666
shouldWarn
57-
? `Warning: Middleware for RTK-Query API at reducerPath "api1" has not been added to the store.
67+
? `Warning: Middleware for RTK-Query API at reducerPath "api" has not been added to the store.
5868
Features like automatic cache collection, automatic refetching etc. will not be available.`
5969
: ''
6070
)
6171
})
6272

6373
test('does not warn if middleware is not missing', () => {
6474
const store = configureStore({
65-
reducer: { api1: api1.reducer },
75+
reducer: { [api1.reducerPath]: api1.reducer },
6676
middleware: (gdm) => gdm().concat(api1.middleware),
6777
})
6878
store.dispatch(api1.endpoints.q1.initiate(undefined))
6979
expect(getLog().log).toBe(``)
7080
})
7181

7282
test('warns only once per api', () => {
73-
const store = configureStore({ reducer: { api1: api1.reducer } })
83+
const store = configureStore({
84+
reducer: { [api1.reducerPath]: api1.reducer },
85+
})
7486
store.dispatch(api1.endpoints.q1.initiate(undefined))
7587
store.dispatch(api1.endpoints.q1.initiate(undefined))
7688
expect(getLog().log)
77-
.toBe(`Warning: Middleware for RTK-Query API at reducerPath "api1" has not been added to the store.
89+
.toBe(`Warning: Middleware for RTK-Query API at reducerPath "api" has not been added to the store.
7890
Features like automatic cache collection, automatic refetching etc. will not be available.`)
7991
})
8092

8193
test('warns multiple times for multiple apis', () => {
8294
const store = configureStore({
83-
reducer: { api1: api1.reducer, api2: api2.reducer },
95+
reducer: {
96+
[api1.reducerPath]: api1.reducer,
97+
[api2.reducerPath]: api2.reducer,
98+
},
8499
})
85100
store.dispatch(api1.endpoints.q1.initiate(undefined))
86101
store.dispatch(api2.endpoints.q1.initiate(undefined))
87102
expect(getLog().log)
88-
.toBe(`Warning: Middleware for RTK-Query API at reducerPath "api1" has not been added to the store.
103+
.toBe(`Warning: Middleware for RTK-Query API at reducerPath "api" has not been added to the store.
89104
Features like automatic cache collection, automatic refetching etc. will not be available.
90105
Warning: Middleware for RTK-Query API at reducerPath "api2" has not been added to the store.
91106
Features like automatic cache collection, automatic refetching etc. will not be available.`)
@@ -117,15 +132,15 @@ describe('missing reducer', () => {
117132
api1.endpoints.q1.select(undefined)(store.getState())
118133
expect(getLog().log).toBe(
119134
shouldWarn
120-
? 'Error: No data found at `state.api1`. Did you forget to add the reducer to the store?'
135+
? 'Error: No data found at `state.api`. Did you forget to add the reducer to the store?'
121136
: ''
122137
)
123138
})
124139
})
125140

126141
test('does not warn if reducer is not missing', () => {
127142
const store = configureStore({
128-
reducer: { api1: api1.reducer },
143+
reducer: { [api1.reducerPath]: api1.reducer },
129144
middleware: (gdm) => gdm().concat(api1.middleware),
130145
})
131146
api1.endpoints.q1.select(undefined)(store.getState())
@@ -143,7 +158,7 @@ describe('missing reducer', () => {
143158
// @ts-expect-error
144159
api1.endpoints.q1.select(undefined)(store.getState())
145160
expect(getLog().log).toBe(
146-
'Error: No data found at `state.api1`. Did you forget to add the reducer to the store?'
161+
'Error: No data found at `state.api`. Did you forget to add the reducer to the store?'
147162
)
148163
})
149164

@@ -158,7 +173,7 @@ describe('missing reducer', () => {
158173
// @ts-expect-error
159174
api2.endpoints.q1.select(undefined)(store.getState())
160175
expect(getLog().log).toBe(
161-
'Error: No data found at `state.api1`. Did you forget to add the reducer to the store?\nError: No data found at `state.api2`. Did you forget to add the reducer to the store?'
176+
'Error: No data found at `state.api`. Did you forget to add the reducer to the store?\nError: No data found at `state.api2`. Did you forget to add the reducer to the store?'
162177
)
163178
})
164179
})
@@ -171,10 +186,91 @@ test('warns only for reducer if everything is missing', async () => {
171186
api1.endpoints.q1.select(undefined)(store.getState())
172187
await store.dispatch(api1.endpoints.q1.initiate(undefined))
173188
expect(getLog().log).toBe(
174-
'Error: No data found at `state.api1`. Did you forget to add the reducer to the store?'
189+
'Error: No data found at `state.api`. Did you forget to add the reducer to the store?'
175190
)
176191
})
177192

193+
describe('warns on multiple apis using the same `reducerPath`', () => {
194+
test('common: two apis, same order', async () => {
195+
const store = configureStore({
196+
reducer: {
197+
[api1.reducerPath]: api1.reducer,
198+
[api1_2.reducerPath]: api1_2.reducer,
199+
},
200+
middleware: (gDM) => gDM().concat(api1.middleware, api1_2.middleware),
201+
})
202+
await store.dispatch(api1.endpoints.q1.initiate(undefined))
203+
// only second api prints
204+
expect(getLog().log).toBe(
205+
`There is a mismatch between slice and middleware for the reducerPath "api".
206+
You can only have one api per reducer path, this will lead to crashes in various situations!
207+
If you have multiple apis, you *have* to specify the reducerPath option when using createApi!`
208+
)
209+
})
210+
211+
test('common: two apis, opposing order', async () => {
212+
const store = configureStore({
213+
reducer: {
214+
[api1.reducerPath]: api1.reducer,
215+
[api1_2.reducerPath]: api1_2.reducer,
216+
},
217+
middleware: (gDM) => gDM().concat(api1_2.middleware, api1.middleware),
218+
})
219+
await store.dispatch(api1.endpoints.q1.initiate(undefined))
220+
// both apis print
221+
expect(getLog().log).toBe(
222+
`There is a mismatch between slice and middleware for the reducerPath "api".
223+
You can only have one api per reducer path, this will lead to crashes in various situations!
224+
If you have multiple apis, you *have* to specify the reducerPath option when using createApi!
225+
There is a mismatch between slice and middleware for the reducerPath "api".
226+
You can only have one api per reducer path, this will lead to crashes in various situations!
227+
If you have multiple apis, you *have* to specify the reducerPath option when using createApi!`
228+
)
229+
})
230+
231+
test('common: two apis, only first middleware', async () => {
232+
const store = configureStore({
233+
reducer: {
234+
[api1.reducerPath]: api1.reducer,
235+
[api1_2.reducerPath]: api1_2.reducer,
236+
},
237+
middleware: (gDM) => gDM().concat(api1.middleware),
238+
})
239+
await store.dispatch(api1.endpoints.q1.initiate(undefined))
240+
241+
expect(getLog().log).toBe(
242+
`There is a mismatch between slice and middleware for the reducerPath "api".
243+
You can only have one api per reducer path, this will lead to crashes in various situations!
244+
If you have multiple apis, you *have* to specify the reducerPath option when using createApi!`
245+
)
246+
})
247+
248+
/**
249+
* This is the one edge case that we currently cannot detect:
250+
* Multiple apis with the same reducer key and only the middleware of the last api is being used.
251+
*
252+
* It would be great to support this case as well, but for now:
253+
* "It is what it is."
254+
*/
255+
test.skip('common: two apis, only second middleware', async () => {
256+
const store = configureStore({
257+
reducer: {
258+
[api1.reducerPath]: api1.reducer,
259+
[api1_2.reducerPath]: api1_2.reducer,
260+
},
261+
middleware: (gDM) => gDM().concat(api1_2.middleware),
262+
})
263+
await store.dispatch(api1.endpoints.q1.initiate(undefined))
264+
265+
expect(getLog().log).toBe(
266+
`There is a mismatch between slice and middleware for the reducerPath "api".
267+
You can only have one api per reducer path, this will lead to crashes in various situations!
268+
If you have multiple apis, you *have* to specify the reducerPath option when using createApi!`
269+
)
270+
})
271+
})
272+
273+
178274
describe('`console.error` on unhandled errors during `initiate`', () => {
179275
test('error thrown in `baseQuery`', async () => {
180276
const api = createApi({
@@ -248,7 +344,7 @@ In the case of an unhandled error, no tags will be "provided" or "invalidated".
248344
test('`fetchBaseQuery`: error thrown in `prepareHeaders`', async () => {
249345
const api = createApi({
250346
baseQuery: fetchBaseQuery({
251-
baseUrl: 'http://example.com',
347+
baseUrl,
252348
prepareHeaders() {
253349
throw new Error('this was kinda expected')
254350
},
@@ -275,7 +371,7 @@ In the case of an unhandled error, no tags will be "provided" or "invalidated".
275371
test('`fetchBaseQuery`: error thrown in `validateStatus`', async () => {
276372
const api = createApi({
277373
baseQuery: fetchBaseQuery({
278-
baseUrl: 'http://example.com',
374+
baseUrl
279375
}),
280376
endpoints: (build) => ({
281377
val: build.query<any, void>({

0 commit comments

Comments
 (0)