Only warn for unexpected key once per key #1818

aweary · 2016-06-17T21:47:03Z

Resolves #1748

Pretty simple, just keeps a caches and filters keys that have already been warned about.

I had to update one of the related tests as it expected some keys to be warned about multiple times.

gaearon · 2016-06-21T21:48:03Z

src/combineReducers.js

@@ -34,7 +36,9 @@ function getUnexpectedStateShapeWarningMessage(inputState, reducers, action) {
    )
  }

-  var unexpectedKeys = Object.keys(inputState).filter(key => !reducers.hasOwnProperty(key))
+  var unexpectedKeys = Object.keys(inputState).filter(key => !reducers.hasOwnProperty(key) && !unexpectedKeyCache[key])


Let’s split this into several lines so they format more nicely?

gaearon · 2016-06-21T21:48:33Z

Looks good to me after fixing the nits.

aweary · 2016-06-21T22:04:42Z

@gaearon the nits have been fixed 👍

gaearon · 2016-06-22T01:25:57Z

src/combineReducers.js

@@ -2,6 +2,8 @@ import { ActionTypes } from './createStore'
 import isPlainObject from 'lodash/isPlainObject'
 import warning from './utils/warning'

+const unexpectedKeyCache = {}


Let’s use var for consistency (we’ll codemod ’em all over later)

aweary · 2016-06-22T01:32:20Z

@gaearon changes made 👍

gaearon · 2016-06-22T13:24:00Z

test/combineReducers.spec.js

@@ -237,5 +237,26 @@ describe('Utils', () => {

      spy.restore()
    })
+
+    it('only warns for unexpected keys once', () => {


Do I undestand correctly that tests now depend on global shared state and run order? This doesn’t look very good.

Right, that should be resolved by using individual cache instances per combineReducers call

aweary · 2016-06-22T19:19:55Z

@gaearon I updated the PR so that it creates and passes a new cache to getUnexpectedStateShapeWarningMessage whenever combineReducers is called. I didn't see any existing code that indicated how you'd want to format these functions now that the argument list is rather long, so let me know if any changes are required 👍

gaearon · 2016-06-22T21:03:26Z

src/combineReducers.js

@@ -101,6 +99,7 @@ function assertReducerSanity(reducers) {
 */
 export default function combineReducers(reducers) {
  var reducerKeys = Object.keys(reducers)
+  var unexpectedKeyCache = {}


Probably no need to declare it unless we’re in DEV.

Create key cache in combineReducers Only define unexpectedKeyCache in dev environment

aweary · 2016-06-22T21:11:52Z

@gaearon updated again. Fourth try's the charm.

gaearon · 2016-06-22T21:34:57Z

Have you tested this on a real project to check that it behaves as expected?

aweary · 2016-06-22T21:57:48Z

I haven't tested it any real project, no. I can run it on some of the example projects and verify the results if you like.

aweary · 2016-06-22T23:09:37Z

@gaearon

I tested the real-world example branch and verified it's working as expected. I changed the rootReducer call so it passed an invalid foobar value to the pagination reducer

const rootReducer = combineReducers({
  entities,
  pagination: (state, action) => {
    return pagination(
      Object.assign({}, state, {foobar: false}),
      action
    )
  },
  errorMessage,
  routing
})

With the current release of redux this caused multiple warnings. After running npm run build and replacing the lib folder in the example project's node_modules/redux folder the error was only logged once no matter what actions were performed.

gaearon · 2016-06-23T00:13:07Z

👍

Create key cache in combineReducers Only define unexpectedKeyCache in dev environment

aweary force-pushed the warn-unexpected-keys-once branch from e016021 to c9b5c3a Compare June 17, 2016 21:55

gaearon reviewed Jun 21, 2016
View reviewed changes

aweary force-pushed the warn-unexpected-keys-once branch from c9b5c3a to 0a3eee0 Compare June 21, 2016 22:04

gaearon reviewed Jun 22, 2016
View reviewed changes

aweary force-pushed the warn-unexpected-keys-once branch from 0a3eee0 to 2c48641 Compare June 22, 2016 01:31

gaearon reviewed Jun 22, 2016
View reviewed changes

Only warn for unexpected key once per key

2d6316d

Create key cache in combineReducers Only define unexpectedKeyCache in dev environment

aweary force-pushed the warn-unexpected-keys-once branch from 13ddb17 to 2d6316d Compare June 22, 2016 21:11

gaearon added the enhancement label Jun 23, 2016

gaearon merged commit ea43078 into reduxjs:master Jun 23, 2016

seantcoyote pushed a commit to seantcoyote/redux that referenced this pull request Jan 14, 2018

Only warn for unexpected key once per key (reduxjs#1818)

09866aa

Create key cache in combineReducers Only define unexpectedKeyCache in dev environment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Only warn for unexpected key once per key #1818

Only warn for unexpected key once per key #1818

aweary commented Jun 17, 2016

gaearon Jun 21, 2016

gaearon commented Jun 21, 2016

aweary commented Jun 21, 2016

gaearon Jun 22, 2016

aweary commented Jun 22, 2016

gaearon Jun 22, 2016

aweary Jun 22, 2016

gaearon Jun 22, 2016

aweary commented Jun 22, 2016

gaearon Jun 22, 2016

aweary commented Jun 22, 2016

gaearon commented Jun 22, 2016

aweary commented Jun 22, 2016 •

edited

Loading

aweary commented Jun 22, 2016

gaearon commented Jun 23, 2016

Only warn for unexpected key once per key #1818

Only warn for unexpected key once per key #1818

Conversation

aweary commented Jun 17, 2016

gaearon Jun 21, 2016

Choose a reason for hiding this comment

gaearon commented Jun 21, 2016

aweary commented Jun 21, 2016

gaearon Jun 22, 2016

Choose a reason for hiding this comment

aweary commented Jun 22, 2016

gaearon Jun 22, 2016

Choose a reason for hiding this comment

aweary Jun 22, 2016

Choose a reason for hiding this comment

gaearon Jun 22, 2016

Choose a reason for hiding this comment

aweary commented Jun 22, 2016

gaearon Jun 22, 2016

Choose a reason for hiding this comment

aweary commented Jun 22, 2016

gaearon commented Jun 22, 2016

aweary commented Jun 22, 2016 • edited Loading

aweary commented Jun 22, 2016

gaearon commented Jun 23, 2016

aweary commented Jun 22, 2016 •

edited

Loading