refactor: remove immutable from medias state slice (#4740)

packages/netlify-cms-core/package.json

@@ -86,6 +86,7 @@
     "react-immutable-proptypes": "^2.1.0"
   },
   "devDependencies": {
+    "@types/redux-mock-store": "^1.0.2",
     "redux-mock-store": "^1.5.3"
   }
 }

packages/netlify-cms-core/src/actions/__tests__/media.spec.ts

@@ -1,11 +1,18 @@
 import { Map } from 'immutable';
-import { getAsset, ADD_ASSET, LOAD_ASSET_REQUEST } from '../media';
 import configureMockStore from 'redux-mock-store';
-import thunk from 'redux-thunk';
+import thunk, { ThunkDispatch } from 'redux-thunk';
+import { AnyAction } from 'redux';
+import { mocked } from 'ts-jest/utils';
+import { getAsset, ADD_ASSET, LOAD_ASSET_REQUEST } from '../media';
+import { selectMediaFilePath } from '../../reducers/entries';
+import { State } from '../../types/redux';
 import AssetProxy from '../../valueObjects/AssetProxy';
 
 const middlewares = [thunk];
-const mockStore = configureMockStore(middlewares);
+const mockStore = configureMockStore<Partial<State>, ThunkDispatch<State, {}, AnyAction>>(
+  middlewares,
+);
+const mockedSelectMediaFilePath = mocked(selectMediaFilePath);
 
 jest.mock('../../reducers/entries');
 jest.mock('../mediaLibrary');
@@ -19,10 +26,10 @@ describe('media', () => {
   });
 
   describe('getAsset', () => {
+    // eslint-disable-next-line @typescript-eslint/ban-ts-ignore
+    // @ts-ignore
     global.URL = { createObjectURL: jest.fn() };
 
-    const { selectMediaFilePath } = require('../../reducers/entries');
-
     beforeEach(() => {
       jest.resetAllMocks();
     });
@@ -30,8 +37,12 @@ describe('media', () => {
     it('should return empty asset for null path', () => {
       const store = mockStore({});
 
-      const payload = { collection: null, entryPath: null, path: null };
+      const payload = { collection: null, entryPath: null, entry: null, path: null };
 
+      // TODO change to proper payload when immutable is removed
+      //  from 'collections' and 'entries' state slices
+      // eslint-disable-next-line @typescript-eslint/ban-ts-ignore
+      // @ts-ignore
       const result = store.dispatch(getAsset(payload));
       const actions = store.getActions();
       expect(actions).toHaveLength(0);
@@ -42,22 +53,30 @@ describe('media', () => {
       const path = 'static/media/image.png';
       const asset = new AssetProxy({ file: new File([], 'empty'), path });
       const store = mockStore({
+        // TODO change to proper store data when immutable is removed
+        //  from 'config' state slice
+        // eslint-disable-next-line @typescript-eslint/ban-ts-ignore
+        // @ts-ignore
         config: Map(),
-        medias: Map({
-          [path]: { asset },
-        }),
+        medias: {
+          [path]: { asset, isLoading: false, error: null },
+        },
       });
 
-      selectMediaFilePath.mockReturnValue(path);
+      mockedSelectMediaFilePath.mockReturnValue(path);
       const payload = { collection: Map(), entry: Map({ path: 'entryPath' }), path };
 
+      // TODO change to proper payload when immutable is removed
+      //  from 'collections' and 'entries' state slices
+      // eslint-disable-next-line @typescript-eslint/ban-ts-ignore
+      // @ts-ignore
       const result = store.dispatch(getAsset(payload));
       const actions = store.getActions();
       expect(actions).toHaveLength(0);
 
       expect(result).toBe(asset);
-      expect(selectMediaFilePath).toHaveBeenCalledTimes(1);
-      expect(selectMediaFilePath).toHaveBeenCalledWith(
+      expect(mockedSelectMediaFilePath).toHaveBeenCalledTimes(1);
+      expect(mockedSelectMediaFilePath).toHaveBeenCalledWith(
         store.getState().config,
         payload.collection,
         payload.entry,
@@ -71,12 +90,16 @@ describe('media', () => {
 
       const asset = new AssetProxy({ url: path, path });
       const store = mockStore({
-        medias: Map({}),
+        medias: {},
       });
 
-      selectMediaFilePath.mockReturnValue(path);
+      mockedSelectMediaFilePath.mockReturnValue(path);
       const payload = { collection: null, entryPath: null, path };
 
+      // TODO change to proper payload when immutable is removed
+      //  from 'collections' state slice
+      // eslint-disable-next-line @typescript-eslint/ban-ts-ignore
+      // @ts-ignore
       const result = store.dispatch(getAsset(payload));
       const actions = store.getActions();
       expect(actions).toHaveLength(1);
@@ -90,12 +113,16 @@ describe('media', () => {
     it('should return empty asset and initiate load when not in medias state', () => {
       const path = 'static/media/image.png';
       const store = mockStore({
-        medias: Map({}),
+        medias: {},
       });
 
-      selectMediaFilePath.mockReturnValue(path);
+      mockedSelectMediaFilePath.mockReturnValue(path);
       const payload = { path };
 
+      // TODO change to proper payload when immutable is removed
+      //  from 'collections' and 'entries' state slices
+      // eslint-disable-next-line @typescript-eslint/ban-ts-ignore
+      // @ts-ignore
       const result = store.dispatch(getAsset(payload));
       const actions = store.getActions();
       expect(actions).toHaveLength(1);
@@ -110,12 +137,22 @@ describe('media', () => {
       const path = 'static/media/image.png';
       const resolvePath = 'resolvePath';
       const store = mockStore({
-        medias: Map({ [resolvePath]: { error: true } }),
+        medias: {
+          [resolvePath]: {
+            asset: undefined,
+            error: new Error('test'),
+            isLoading: false,
+          },
+        },
       });
 
-      selectMediaFilePath.mockReturnValue(resolvePath);
+      mockedSelectMediaFilePath.mockReturnValue(resolvePath);
       const payload = { path };
 
+      // TODO change to proper payload when immutable is removed
+      //  from 'collections' and 'entries' state slices
+      // eslint-disable-next-line @typescript-eslint/ban-ts-ignore
+      // @ts-ignore
       const result = store.dispatch(getAsset(payload));
       const actions = store.getActions();
 

packages/netlify-cms-core/src/actions/media.ts

@@ -16,27 +16,27 @@ export const LOAD_ASSET_SUCCESS = 'LOAD_ASSET_SUCCESS';
 export const LOAD_ASSET_FAILURE = 'LOAD_ASSET_FAILURE';
 
 export function addAssets(assets: AssetProxy[]) {
-  return { type: ADD_ASSETS, payload: assets };
+  return { type: ADD_ASSETS, payload: assets } as const;
 }
 
 export function addAsset(assetProxy: AssetProxy) {
-  return { type: ADD_ASSET, payload: assetProxy };
+  return { type: ADD_ASSET, payload: assetProxy } as const;
 }
 
 export function removeAsset(path: string) {
-  return { type: REMOVE_ASSET, payload: path };
+  return { type: REMOVE_ASSET, payload: path } as const;
 }
 
 export function loadAssetRequest(path: string) {
-  return { type: LOAD_ASSET_REQUEST, payload: { path } };
+  return { type: LOAD_ASSET_REQUEST, payload: { path } } as const;
 }
 
 export function loadAssetSuccess(path: string) {
-  return { type: LOAD_ASSET_SUCCESS, payload: { path } };
+  return { type: LOAD_ASSET_SUCCESS, payload: { path } } as const;
 }
 
 export function loadAssetFailure(path: string, error: Error) {
-  return { type: LOAD_ASSET_FAILURE, payload: { path, error } };
+  return { type: LOAD_ASSET_FAILURE, payload: { path, error } } as const;
 }
 
 export function loadAsset(resolvedPath: string) {
@@ -97,7 +97,7 @@ export function getAsset({ collection, entry, path, field }: GetAssetArgs) {
     const state = getState();
     const resolvedPath = selectMediaFilePath(state.config, collection, entry, path, field);
 
-    let { asset, isLoading, error } = state.medias.get(resolvedPath) || {};
+    let { asset, isLoading, error } = state.medias[resolvedPath] || {};
     if (isLoading) {
       return emptyAsset;
     }
@@ -125,3 +125,12 @@ export function getAsset({ collection, entry, path, field }: GetAssetArgs) {
     return asset;
   };
 }
+
+export type MediasAction = ReturnType<
+  | typeof addAssets
+  | typeof addAsset
+  | typeof removeAsset
+  | typeof loadAssetRequest
+  | typeof loadAssetSuccess
+  | typeof loadAssetFailure
+>;

packages/netlify-cms-core/src/reducers/__tests__/medias.spec.ts

@@ -1,4 +1,3 @@
-import { Map, fromJS } from 'immutable';
 import {
   addAssets,
   addAsset,
@@ -14,37 +13,37 @@ describe('medias', () => {
   const asset = createAssetProxy({ url: 'url', path: 'path' });
 
   it('should add assets', () => {
-    expect(reducer(fromJS({}), addAssets([asset]))).toEqual(
-      Map({ path: { asset, isLoading: false, error: null } }),
-    );
+    expect(reducer({}, addAssets([asset]))).toEqual({
+      path: { asset, isLoading: false, error: null },
+    });
   });
 
   it('should add asset', () => {
-    expect(reducer(fromJS({}), addAsset(asset))).toEqual(
-      Map({ path: { asset, isLoading: false, error: null } }),
-    );
+    expect(reducer({}, addAsset(asset))).toEqual({
+      path: { asset, isLoading: false, error: null },
+    });
   });
 
   it('should remove asset', () => {
-    expect(reducer(fromJS({ path: asset }), removeAsset(asset.path))).toEqual(Map());
+    expect(
+      reducer({ [asset.path]: { asset, isLoading: false, error: null } }, removeAsset(asset.path)),
+    ).toEqual({});
   });
 
   it('should mark asset as loading', () => {
-    expect(reducer(fromJS({}), loadAssetRequest(asset.path))).toEqual(
-      Map({ path: { isLoading: true } }),
-    );
+    expect(reducer({}, loadAssetRequest(asset.path))).toEqual({ path: { isLoading: true } });
   });
 
   it('should mark asset as not loading', () => {
-    expect(reducer(fromJS({}), loadAssetSuccess(asset.path))).toEqual(
-      Map({ path: { isLoading: false, error: null } }),
-    );
+    expect(reducer({}, loadAssetSuccess(asset.path))).toEqual({
+      path: { isLoading: false, error: null },
+    });
   });
 
   it('should set loading error', () => {
     const error = new Error('some error');
-    expect(reducer(fromJS({}), loadAssetFailure(asset.path, error))).toEqual(
-      Map({ path: { isLoading: false, error } }),
-    );
+    expect(reducer({}, loadAssetFailure(asset.path, error))).toEqual({
+      path: { isLoading: false, error },
+    });
   });
 });

packages/netlify-cms-core/src/reducers/medias.ts

@@ -1,4 +1,4 @@
-import { fromJS } from 'immutable';
+import { produce } from 'immer';
 import {
   ADD_ASSETS,
   ADD_ASSET,
@@ -6,46 +6,58 @@ import {
   LOAD_ASSET_REQUEST,
   LOAD_ASSET_SUCCESS,
   LOAD_ASSET_FAILURE,
+  MediasAction,
 } from '../actions/media';
 import AssetProxy from '../valueObjects/AssetProxy';
-import { Medias, MediasAction } from '../types/redux';
 
-const medias = (state: Medias = fromJS({}), action: MediasAction) => {
-  switch (action.type) {
-    case ADD_ASSETS: {
-      const payload = action.payload as AssetProxy[];
-      let newState = state;
-      payload.forEach(asset => {
-        newState = newState.set(asset.path, { asset, isLoading: false, error: null });
-      });
-      return newState;
-    }
-    case ADD_ASSET: {
-      const asset = action.payload as AssetProxy;
-      return state.set(asset.path, { asset, isLoading: false, error: null });
-    }
-    case REMOVE_ASSET: {
-      const payload = action.payload as string;
-      return state.delete(payload);
-    }
-    case LOAD_ASSET_REQUEST: {
-      const { path } = action.payload as { path: string };
-      return state.set(path, { ...state.get(path), isLoading: true });
-    }
-    case LOAD_ASSET_SUCCESS: {
-      const { path } = action.payload as { path: string };
-      return state.set(path, { ...state.get(path), isLoading: false, error: null });
-    }
-    case LOAD_ASSET_FAILURE: {
-      const { path, error } = action.payload as { path: string; error: Error };
-      return state.set(path, { ...state.get(path), isLoading: false, error });
-    }
-    default:
-      return state;
-  }
+export type Medias = {
+  [path: string]: { asset: AssetProxy | undefined; isLoading: boolean; error: Error | null };
 };
 
+const defaultState: Medias = {};
+
+const medias = produce((state: Medias, action: MediasAction) => {
+  switch (action.type) {
+    case ADD_ASSETS: {
+      const assets = action.payload;
+      assets.forEach(asset => {
+        state[asset.path] = { asset, isLoading: false, error: null };
+      });
+      break;
+    }
+    case ADD_ASSET: {
+      const asset = action.payload;
+      state[asset.path] = { asset, isLoading: false, error: null };
+      break;
+    }
+    case REMOVE_ASSET: {
+      const path = action.payload;
+      delete state[path];
+      break;
+    }
+    case LOAD_ASSET_REQUEST: {
+      const { path } = action.payload;
+      state[path] = state[path] || {};
+      state[path].isLoading = true;
+      break;
+    }
+    case LOAD_ASSET_SUCCESS: {
+      const { path } = action.payload;
+      state[path] = state[path] || {};
+      state[path].isLoading = false;
+      state[path].error = null;
+      break;
+    }
+    case LOAD_ASSET_FAILURE: {
+      const { path, error } = action.payload;
+      state[path] = state[path] || {};
+      state[path].isLoading = false;
+      state[path].error = error;
+    }
+  }
+}, defaultState);
+
 export const selectIsLoadingAsset = (state: Medias) =>
-  Object.values(state.toJS()).some(state => state.isLoading);
+  Object.values(state).some(state => state.isLoading);
 
 export default medias;

packages/netlify-cms-core/src/types/redux.ts

@@ -1,10 +1,10 @@
 import { Action } from 'redux';
 import { StaticallyTypedRecord } from './immutable';
 import { Map, List, OrderedMap, Set } from 'immutable';
-import AssetProxy from '../valueObjects/AssetProxy';
 import { MediaFile as BackendMediaFile } from '../backend';
 import { Auth } from '../reducers/auth';
 import { Status } from '../reducers/status';
+import { Medias } from '../reducers/medias';
 
 export type SlugConfig = StaticallyTypedRecord<{
   encoding: string;
@@ -226,10 +226,6 @@ export type Collection = StaticallyTypedRecord<CollectionObject>;
 
 export type Collections = StaticallyTypedRecord<{ [path: string]: Collection & CollectionObject }>;
 
-export type Medias = StaticallyTypedRecord<{
-  [path: string]: { asset: AssetProxy | undefined; isLoading: boolean; error: Error | null };
-}>;
-
 export interface MediaLibraryInstance {
   show: (args: {
     id?: string;
@@ -308,10 +304,6 @@ export interface State {
   status: Status;
 }
 
-export interface MediasAction extends Action<string> {
-  payload: string | AssetProxy | AssetProxy[] | { path: string } | { path: string; error: Error };
-}
-
 export interface ConfigAction extends Action<string> {
   payload: Map<string, boolean>;
 }

yarn.lock

@@ -3484,6 +3484,13 @@
     "@types/prop-types" "*"
     csstype "^3.0.2"
 
+"@types/redux-mock-store@^1.0.2":
+  version "1.0.2"
+  resolved "https://registry.yarnpkg.com/@types/redux-mock-store/-/redux-mock-store-1.0.2.tgz#c27d5deadfb29d8514bdb0fc2cadae6feea1922d"
+  integrity sha512-6LBtAQBN34i7SI5X+Qs4zpTEZO1tTDZ6sZ9fzFjYwTl3nLQXaBtwYdoV44CzNnyKu438xJ1lSIYyw0YMvunESw==
+  dependencies:
+    redux "^4.0.5"
+
 "@types/serve-static@*":
   version "1.13.8"
   resolved "https://registry.yarnpkg.com/@types/serve-static/-/serve-static-1.13.8.tgz#851129d434433c7082148574ffec263d58309c46"