Added deep freeze of stores in dev mode (#135)
* Added deep freeze of stores in dev mode * Stopped deletePost and reducePosts reducer functions from mutating the store
This commit is contained in:
parent
fcd86b8923
commit
bd3bd45c36
5 changed files with 128 additions and 50 deletions
|
|
@ -43,7 +43,7 @@ export function deletePost(teamId, post) {
|
|||
dispatch(batchActions([
|
||||
{
|
||||
type: PostsTypes.POST_DELETED,
|
||||
data: post
|
||||
data: {...post}
|
||||
},
|
||||
{
|
||||
type: PostsTypes.DELETE_POST_SUCCESS
|
||||
|
|
@ -56,7 +56,7 @@ export function removePost(post) {
|
|||
return async (dispatch, getState) => {
|
||||
dispatch({
|
||||
type: PostsTypes.REMOVE_POST,
|
||||
data: post
|
||||
data: {...post}
|
||||
}, getState);
|
||||
};
|
||||
}
|
||||
|
|
@ -77,7 +77,7 @@ export function getPost(teamId, channelId, postId) {
|
|||
dispatch(batchActions([
|
||||
{
|
||||
type: PostsTypes.RECEIVED_POSTS,
|
||||
data: post,
|
||||
data: {...post},
|
||||
channelId
|
||||
},
|
||||
{
|
||||
|
|
|
|||
|
|
@ -111,11 +111,14 @@ function deletePost(state, action) {
|
|||
if (postInfo.posts[post.id]) {
|
||||
const combinedPosts = makePostListNonNull(postInfo);
|
||||
|
||||
combinedPosts.posts[post.id] = {
|
||||
...post,
|
||||
state: Constants.POST_DELETED,
|
||||
file_ids: [],
|
||||
has_reactions: false
|
||||
combinedPosts.posts = {
|
||||
...combinedPosts.posts,
|
||||
[post.id]: {
|
||||
...post,
|
||||
state: Constants.POST_DELETED,
|
||||
file_ids: [],
|
||||
has_reactions: false
|
||||
}
|
||||
};
|
||||
|
||||
return {
|
||||
|
|
@ -143,6 +146,11 @@ function removePost(state, action) {
|
|||
|
||||
if (postInfo.posts[post.id]) {
|
||||
const combinedPosts = makePostListNonNull(postInfo);
|
||||
|
||||
combinedPosts.order = combinedPosts.order.slice(0);
|
||||
combinedPosts.posts = {...combinedPosts.posts};
|
||||
|
||||
// Remove the post
|
||||
Reflect.deleteProperty(combinedPosts.posts, post.id);
|
||||
|
||||
const index = combinedPosts.order.indexOf(post.id);
|
||||
|
|
@ -150,11 +158,8 @@ function removePost(state, action) {
|
|||
combinedPosts.order.splice(index, 1);
|
||||
}
|
||||
|
||||
for (const pid in combinedPosts.posts) {
|
||||
if (!combinedPosts.posts.hasOwnProperty(pid)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Remove any children of the post
|
||||
for (const pid of Object.keys(combinedPosts.posts)) {
|
||||
if (combinedPosts.posts[pid].root_id === post.id) {
|
||||
Reflect.deleteProperty(combinedPosts.posts, pid);
|
||||
const commentIndex = combinedPosts.order.indexOf(pid);
|
||||
|
|
|
|||
|
|
@ -2,14 +2,16 @@
|
|||
// See License.txt for license information.
|
||||
|
||||
import {applyMiddleware, compose, createStore, combineReducers} from 'redux';
|
||||
import devTools from 'remote-redux-devtools';
|
||||
import {enableBatching} from 'redux-batched-actions';
|
||||
import serviceReducer from 'service/reducers';
|
||||
import devTools from 'remote-redux-devtools';
|
||||
import thunk from 'redux-thunk';
|
||||
|
||||
import serviceReducer from 'service/reducers';
|
||||
import deepFreezeAndThrowOnMutation from 'service/utils/deepFreezeAndThrowOnMutation';
|
||||
|
||||
export default function configureServiceStore(preloadedState, appReducer, getAppReducer) {
|
||||
const store = createStore(
|
||||
enableBatching(combineReducers(Object.assign({}, serviceReducer, appReducer))),
|
||||
createReducer(serviceReducer, appReducer),
|
||||
preloadedState,
|
||||
compose(
|
||||
applyMiddleware(thunk),
|
||||
|
|
@ -29,9 +31,27 @@ export default function configureServiceStore(preloadedState, appReducer, getApp
|
|||
if (getAppReducer) {
|
||||
nextAppReducer = getAppReducer(); // eslint-disable-line global-require
|
||||
}
|
||||
store.replaceReducer(combineReducers(Object.assign({}, nextServiceReducer, nextAppReducer)));
|
||||
store.replaceReducer(createReducer(nextServiceReducer, nextAppReducer));
|
||||
});
|
||||
}
|
||||
|
||||
return store;
|
||||
}
|
||||
|
||||
function createReducer(...reducers) {
|
||||
const baseReducer = combineReducers(Object.assign({}, ...reducers));
|
||||
|
||||
return enableFreezing(enableBatching(baseReducer));
|
||||
}
|
||||
|
||||
function enableFreezing(reducer) {
|
||||
return (state, action) => {
|
||||
const nextState = reducer(state, action);
|
||||
|
||||
if (nextState !== state) {
|
||||
deepFreezeAndThrowOnMutation(nextState);
|
||||
}
|
||||
|
||||
return nextState;
|
||||
};
|
||||
}
|
||||
|
|
|
|||
60
service/utils/deepFreezeAndThrowOnMutation.js
Normal file
60
service/utils/deepFreezeAndThrowOnMutation.js
Normal file
|
|
@ -0,0 +1,60 @@
|
|||
/**
|
||||
* Copyright (c) 2015-present, Facebook, Inc.
|
||||
* All rights reserved.
|
||||
*
|
||||
* This source code is licensed under the BSD-style license found in the
|
||||
* LICENSE file in the root directory of this source tree. An additional grant
|
||||
* of patent rights can be found in the PATENTS file in the same directory.
|
||||
*
|
||||
*/
|
||||
|
||||
/**
|
||||
* If your application is accepting different values for the same field over
|
||||
* time and is doing a diff on them, you can either (1) create a copy or
|
||||
* (2) ensure that those values are not mutated behind two passes.
|
||||
* This function helps you with (2) by freezing the object and throwing if
|
||||
* the user subsequently modifies the value.
|
||||
*
|
||||
* There are two caveats with this function:
|
||||
* - If the call site is not in strict mode, it will only throw when
|
||||
* mutating existing fields, adding a new one
|
||||
* will unfortunately fail silently :(
|
||||
* - If the object is already frozen or sealed, it will not continue the
|
||||
* deep traversal and will leave leaf nodes unfrozen.
|
||||
*
|
||||
* Freezing the object and adding the throw mechanism is expensive and will
|
||||
* only be used in DEV.
|
||||
*/
|
||||
export default function deepFreezeAndThrowOnMutation(object) {
|
||||
if (typeof object !== 'object' || object === null || Object.isFrozen(object) || Object.isSealed(object)) {
|
||||
return;
|
||||
}
|
||||
|
||||
for (const key in object) {
|
||||
if (object.hasOwnProperty(key)) {
|
||||
object.__defineGetter__(key, identity.bind(null, object[key])); // eslint-disable-line no-underscore-dangle
|
||||
object.__defineSetter__(key, throwOnImmutableMutation.bind(null, key)); // eslint-disable-line no-underscore-dangle
|
||||
}
|
||||
}
|
||||
|
||||
Object.freeze(object);
|
||||
Object.seal(object);
|
||||
|
||||
for (const key in object) {
|
||||
if (object.hasOwnProperty(key)) {
|
||||
deepFreezeAndThrowOnMutation(object[key]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function throwOnImmutableMutation(key, value) {
|
||||
throw Error(
|
||||
'You attempted to set the key `' + key + '` with the value `' +
|
||||
JSON.stringify(value) + '` on an object that is meant to be immutable ' +
|
||||
'and has been frozen.'
|
||||
);
|
||||
}
|
||||
|
||||
function identity(value) {
|
||||
return value;
|
||||
}
|
||||
|
|
@ -75,45 +75,36 @@ describe('Actions.Posts', () => {
|
|||
});
|
||||
|
||||
it('deletePost', (done) => {
|
||||
TestHelper.initBasic(Client).then(async () => {
|
||||
TestHelper.initBasic(Client).then(async ({channel}) => {
|
||||
const store = configureStore();
|
||||
let created;
|
||||
|
||||
await Actions.createPost(
|
||||
TestHelper.basicTeam.id,
|
||||
TestHelper.fakePost(TestHelper.basicChannel.id)
|
||||
)(store.dispatch, store.getState);
|
||||
|
||||
const initialPostsInfo = store.getState().entities.posts.postsInfo[channel.id];
|
||||
const created = initialPostsInfo.posts[initialPostsInfo.order[0]];
|
||||
|
||||
store.subscribe(() => {
|
||||
const createRequest = store.getState().requests.posts.createPost;
|
||||
const deleteRequest = store.getState().requests.posts.deletePost;
|
||||
const postsInfo = store.getState().entities.posts.postsInfo;
|
||||
const state = store.getState();
|
||||
const deleteRequest = state.requests.posts.deletePost;
|
||||
const postsInfo = state.entities.posts.postsInfo[channel.id];
|
||||
|
||||
if (deleteRequest.status === RequestStatus.SUCCESS || deleteRequest.status === RequestStatus.FAILURE) {
|
||||
if (deleteRequest.error) {
|
||||
done(new Error(JSON.stringify(deleteRequest.error)));
|
||||
} else {
|
||||
assert.ok(postsInfo[TestHelper.basicChannel.id]);
|
||||
assert.strictEqual(
|
||||
postsInfo[TestHelper.basicChannel.id].posts[created.id].state,
|
||||
Constants.POST_DELETED
|
||||
);
|
||||
done();
|
||||
}
|
||||
}
|
||||
if (deleteRequest.status === RequestStatus.SUCCESS) {
|
||||
assert.ok(postsInfo);
|
||||
assert.strictEqual(
|
||||
postsInfo.posts[created.id].state,
|
||||
Constants.POST_DELETED
|
||||
);
|
||||
|
||||
if (deleteRequest.status === RequestStatus.NOT_STARTED &&
|
||||
(createRequest.status === RequestStatus.SUCCESS || createRequest.status === RequestStatus.FAILURE)) {
|
||||
if (createRequest.error) {
|
||||
done(new Error(JSON.stringify(createRequest.error)));
|
||||
} else {
|
||||
const posts = postsInfo[TestHelper.basicChannel.id].posts;
|
||||
created = posts[Object.keys(posts)[0]];
|
||||
Actions.deletePost(TestHelper.basicTeam.id, created)(store.dispatch, store.getState);
|
||||
}
|
||||
done();
|
||||
} else if (deleteRequest.status === RequestStatus.FAILURE) {
|
||||
done(new Error(JSON.stringify(deleteRequest.error)));
|
||||
}
|
||||
});
|
||||
|
||||
const post = TestHelper.fakePost(TestHelper.basicChannel.id);
|
||||
Actions.createPost(
|
||||
TestHelper.basicTeam.id,
|
||||
post
|
||||
)(store.dispatch, store.getState);
|
||||
Actions.deletePost(TestHelper.basicTeam.id, created)(store.dispatch, store.getState);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -133,9 +124,11 @@ describe('Actions.Posts', () => {
|
|||
store.subscribe(() => {
|
||||
const postsInfo = store.getState().entities.posts.postsInfo;
|
||||
const postList = postsInfo[TestHelper.basicChannel.id];
|
||||
|
||||
assert.strictEqual(postList.order.length, 0);
|
||||
assert.ifError(postList.posts[post1a.id]);
|
||||
assert.ifError(postList.posts[TestHelper.basicPost.id]);
|
||||
assert.ok(!postList.posts[post1a.id]);
|
||||
assert.ok(!postList.posts[TestHelper.basicPost.id]);
|
||||
|
||||
done();
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue