Skip to content

Commit efb802b

Browse files
jancajthamljancajthaml
authored
detect module trying to reach in other reducer state and produce warning
1 parent df526e7 commit efb802b

File tree

14 files changed

+272
-126
lines changed

14 files changed

+272
-126
lines changed

‎.github/workflows/test.yml

+10-2
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,22 @@ jobs:
3333
working-directory: platform
3434
run: node ../cli/index.js test
3535
- name: Collect coverage for platform
36-
uses: codecov/codecov-action@v3
36+
uses: codecov/codecov-action@v4
37+
env:
38+
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
3739
with:
40+
fail_ci_if_error: true
41+
flags: unittests
3842
file: platform/coverage/clover.xml
3943
- name: Test package bootstrap
4044
working-directory: bootstrap
4145
run: node ../cli/index.js test
4246
- name: Collect coverage for bootstrap
43-
uses: codecov/codecov-action@v3
47+
uses: codecov/codecov-action@v4
48+
env:
49+
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
4450
with:
51+
fail_ci_if_error: true
52+
flags: unittests
4553
file: bootstrap/coverage/clover.xml
4654

‎bootstrap/src/saga/bootstrap.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ export function* watchBootstrap() {
77
}
88

99
export function* runRefresher(action) {
10-
const interval = Number(action.payload.contextRefreshInterval);
10+
const interval = isNaN(action.payload.contextRefreshInterval) ? 0 : Number(action.payload.contextRefreshInterval);
1111
const predicate = interval > 0 && process.env.NODE_ENV !== "development";
1212
if (predicate) {
1313
console.debug(`context will refresh automatically each ${interval} ms.`);

‎bootstrap/src/selector/index.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
const emptydict = {};
2+
const descriptor = { configurable: true, enumerable: true };
23

34
const memoizedMessages = new Proxy(
45
{
@@ -18,7 +19,7 @@ const memoizedMessages = new Proxy(
1819
next = messages[locale];
1920
}
2021
if (next) {
21-
ref.descriptor = { configurable: true, enumerable: true };
22+
ref.descriptor = descriptor;
2223
ref.messages = next;
2324
} else {
2425
ref.descriptor = undefined;

‎bootstrap/src/store/__test__/index.test.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ describe("store", () => {
55

66
afterAll(() => {
77
process.env.NODE_ENV = ORIGINAL_NODE_ENV;
8-
delete window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__;
8+
delete top.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__;
99
delete global.BUILD_ID;
1010
});
1111

@@ -32,12 +32,12 @@ describe("store", () => {
3232

3333
it("contains window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ in NODE_ENV=development", async () => {
3434
global.BUILD_ID = "xxx";
35-
window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ = jest.fn();
36-
window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__.mockReturnValue(() => {});
35+
top.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ = jest.fn();
36+
top.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__.mockReturnValue(() => {});
3737
process.env.NODE_ENV = "development";
3838
const fetchContext = jest.fn();
3939
const store = await setupStore(fetchContext, []);
4040
expect(store).toBeDefined();
41-
expect(window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__).toHaveBeenCalledWith(expect.objectContaining({ name: "rocker-xxx" }));
41+
expect(top.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__).toHaveBeenCalledWith(expect.objectContaining({ name: "rocker-xxx" }));
4242
});
4343
});

‎bootstrap/src/store/index.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ export default (fetchContext, bootstrapMiddlewares) => {
3434
const enhancers = [loaderMiddleware, sagaMiddleware, ...(bootstrapMiddlewares || []), dynamicMiddleware];
3535

3636
const composer =
37-
process.env.NODE_ENV === "development" && window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__
38-
? window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__({ name: `rocker-${BUILD_ID}` })
37+
process.env.NODE_ENV === "development" && top.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__
38+
? top.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__({ name: `rocker-${BUILD_ID}` })
3939
: compose;
4040

4141
const reducer = combineReducers({

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@lastui/rocker",
3-
"version": "0.19.17",
3+
"version": "0.19.18",
44
"license": "Apache-2.0",
55
"author": "[email protected]",
66
"homepage": "https://github.com/lastui/rocker#readme",

‎platform/src/kernel/reducer/__tests__/modules.test.js

+1-6
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,7 @@ describe("modules reducer", () => {
8181
};
8282

8383
expect(reducer(state, action)).toEqual(expectedState);
84-
expect(spyError).toHaveBeenCalledWith(
85-
"module my-feature-broken reducer failed to reduce on action non-handled",
86-
new Error("ouch"),
87-
);
88-
89-
expect(reducer(state, action)).toEqual(expectedState);
84+
expect(reducer(expectedState, action)).toEqual(expectedState);
9085
expect(spyError).toHaveBeenCalledWith(
9186
"module my-feature-broken reducer failed to reduce on action non-handled",
9287
new Error("ouch"),

‎platform/src/kernel/reducer/modules.js

+23-22
Original file line numberDiff line numberDiff line change
@@ -59,18 +59,17 @@ function createModulesReducer() {
5959
case constants.MODULE_INIT: {
6060
const name = action.payload.module;
6161
console.debug(`module ${name} initialized`);
62-
let changed = false;
6362
const reducer = modulesReducers[name];
64-
if (reducer) {
65-
try {
66-
state[name] = reducer(state[name], action);
67-
changed = true;
68-
} catch (error) {
69-
warning(`module ${name} reducer failed to reduce on action ${action.type}`, error);
70-
}
63+
if (!reducer) {
64+
return state;
7165
}
72-
if (changed) {
73-
return { ...state };
66+
try {
67+
return {
68+
...state,
69+
[name]: reducer(state[name], action),
70+
};
71+
} catch (error) {
72+
warning(`module ${name} reducer failed to reduce on action ${action.type}`, error);
7473
}
7574
return state;
7675
}
@@ -82,34 +81,36 @@ function createModulesReducer() {
8281
}
8382
case constants.MODULE_UNLOADED: {
8483
const name = action.payload.module;
85-
let changed = false;
86-
if (state[name]) {
84+
let changed = name in state;
85+
if (changed) {
8786
console.debug(`module ${name} evicting redux state`);
88-
delete state[name];
89-
changed = true;
9087
}
9188
console.debug(`module ${name} unloaded`);
9289
console.info(`- module ${name}`);
9390
if (changed) {
94-
return { ...state };
91+
const nextState = { ...state };
92+
delete nextState[name];
93+
return nextState;
9594
}
9695
return state;
9796
}
9897
default: {
99-
let changed = false;
98+
let nextState = null;
10099
for (const [name, reducer] of modulesReducers) {
101100
try {
102-
const nextState = reducer(state[name], action);
103-
if (state[name] !== nextState) {
104-
state[name] = nextState;
105-
changed = true;
101+
const fragment = reducer(state[name], action);
102+
if (state[name] !== fragment) {
103+
if (!nextState) {
104+
nextState = { ...state };
105+
}
106+
nextState[name] = fragment;
106107
}
107108
} catch (error) {
108109
warning(`module ${name} reducer failed to reduce on action ${action.type}`, error);
109110
}
110111
}
111-
if (changed) {
112-
return { ...state };
112+
if (nextState) {
113+
return nextState;
113114
}
114115
return state;
115116
}

‎platform/src/kernel/reducer/shared.js

+8-2
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ function createSharedReducer() {
1616
case constants.SET_SHARED: {
1717
if (!action.payload.module || typeof action.payload.module !== "string") {
1818
return {
19-
global: Object.assign({}, state.global, action.payload.data),
19+
global: {
20+
...state.global,
21+
...action.payload.data,
22+
},
2023
local: state.local,
2124
language: state.language,
2225
messages: state.messages,
@@ -25,7 +28,10 @@ function createSharedReducer() {
2528
};
2629
}
2730
const nextLocal = { ...state.local };
28-
nextLocal[action.payload.module] = Object.assign({}, nextLocal[action.payload.module], action.payload.data);
31+
nextLocal[action.payload.module] = {
32+
...nextLocal[action.payload.module],
33+
...action.payload.data,
34+
};
2935
return {
3036
global: state.global,
3137
local: nextLocal,

‎platform/src/kernel/registry/__tests__/reducer.test.js

+9-6
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,17 @@ jest.mock("../../reducer/modules", () => ({
55
}));
66

77
describe("reducer registry", () => {
8-
const debugSpy = jest.spyOn(console, "debug");
9-
debugSpy.mockImplementation(() => {});
8+
const debugSpy = jest.spyOn(console, "debug").mockImplementation(() => {});
9+
const errorSpy = jest.spyOn(console, "error").mockImplementation(() => {});
1010

1111
beforeEach(() => {
1212
debugSpy.mockClear();
13+
errorSpy.mockClear();
14+
});
15+
16+
afterAll(() => {
17+
debugSpy.mockRestore();
18+
errorSpy.mockRestore();
1319
});
1420

1521
it("addReducer", async () => {
@@ -23,15 +29,12 @@ describe("reducer registry", () => {
2329
});
2430
expect(debugSpy).toHaveBeenLastCalledWith("module my-feature replacing reducer");
2531

26-
const spy = jest.spyOn(console, "error");
27-
spy.mockImplementation(() => {});
28-
2932
await addReducer("my-feature", {
3033
bar: (_state, _action) => {
3134
throw new Error("ouch");
3235
},
3336
});
34-
expect(spy).toHaveBeenCalledWith("module my-feature wanted to register invalid reducer", new Error("ouch"));
37+
expect(errorSpy).toHaveBeenCalledWith("module my-feature wanted to register invalid reducer", new Error("ouch"));
3538
});
3639

3740
it("removeReducer", async () => {

0 commit comments

Comments
 (0)