Refactor IDE reducer & actions using Redux Toolkit

[lindapaiste]

Builds upon #2187 and should be merged after.
Associated issue #2042

Explanation:
The Redux Toolkit createSlice function is the modern way to write Redux code, and it offers a lot of advantages over the older approach which is used in this app. You only need to define the reducer in one place instead of three, as the action type constants and action creator functions are generated automatically. This makes the code easier to maintain and it takes a lot less lines (-120 in actions, -43 in constants, +32 in reducer = -131 net).

I am a bit bummed that the action creators don't have good intellisense support, as seen here:

Redux Toolkit is designed to be used with TypeScript and in a pure JS environment it doesn't infer the types properly. The above functions should be ActionCreatorWithoutPayload and require no arguments. But it gets inferred as requiring a payload and shows a false warning in the IDE.

Changes:

• Rewrite the IDE reducer using createSlice.
• Export the auto-generated action creator functions from the actions/ide.js file.
• Delete existing action creator functions which are now auto-generated.
• Use action creators instead of raw action objects when dispatching actions in thunks (I had to do some wonky renaming of showShareModal to showShareModalInternal to support this)
• Delete the corresponding action names from constants.js

I wanted to keep the scope of this PR limited so I only changed the internals of the reducer/actions. All of the exported actions are the same and the structure of the state is the same. I put in a bunch of // TODO comments with things that I think should be changed. These can be tackled in future PRs.

I have verified that this pull request:

• has no linting errors (npm run lint)
• has no test errors (npm run test)
• is from a uniquely-named feature branch and is up to date with the develop branch.
• is descriptively named and links to an issue number, i.e. Fixes #123

[lindapaiste]

Realized I made a mistake on this. I assumed that the actions of each reducer are independent but they are not. Multiple reducers respond to ActionTypes.RESET_PROJECT for example. I will need to change some things before this can be merged.

[lindapaiste]

Realized I made a mistake on this. I assumed that the actions of each reducer are independent but they are not. Multiple reducers respond to ActionTypes.RESET_PROJECT for example. I will need to change some things before this can be merged.

This has been fixed using extraReducers.

[PiyushChandra17]

I think the old redux way was more convenient/concise to read as compared to Redux toolkit, maybe i like that way better.

[raclim]

Thanks for working on this!

I can see how the current version of Redux we're using can feel easier to read and less abstract, but I do appreciate the simplicity that the createSlice function adds! I think I might want to take some more time looking over the "TODOs" listed in this PR, since there's a few of them and I'm wondering if it might be better to address more of them or note them in a separate issue and remove the comments before merging this in. However, I think overall this looks good so far!

Another thing that I noticed happening for me is that this error appears after toggling a few actions. Is this tied to the warning you're mentioning?

[lindapaiste]

I think I might want to take some more time looking over the "TODOs" listed in this PR, since there's a few of them and I'm wondering if it might be better to address more of them or note them in a separate issue and remove the comments before merging this in.

Sure. I'm definitely down to clean up more. I was trying to limit the scope of the PR so I kept the structure of the Redux state identical, even in places where I think it can be improved. Also we can put the TODOs into a checklist somewhere. It was easiest for the to take note of things as I was writing the code.

Another thing that I noticed happening for me is that this error appears after toggling a few actions. Is this tied to the warning you're mentioning?

I'll have to take a look and see what the non-serializable value is. Like if it's in the action or the state and which action it's from. It's possible that I made a mistake in this PR but also possible that the problematic code existed before. The error gets thrown by Redux Toolkit's serializableCheck middleware. It's checking that you are following best practices in your code.

p5.js-web-editor/client/store.js

Lines 27 to 29 in 7e069ee

	serializableCheck: true,
	// TODO: enable immutableCheck once the mutations are fixed.
	immutableCheck: false

I had to disable the immutableCheck check middleware because we don't pass the check, as explained in #2186.

---

Edit: ok so I saw the warning coming from the expandSidebar action. This action creator does not require any arguments. But with the way that it's called in the Editor component, the event gets passed as the payload of the action. The non-serializable value is the Event object which is a class instance.

p5.js-web-editor/client/modules/IDE/components/Editor/index.jsx

Line 530 in 7e069ee

onClick={this.props.expandSidebar}

We can change the onClick to prevent passing any argument:

onClick={() => this.props.expandSidebar()}

It's not an issue in the current code because the action creator which we created manually will ignore any arguments.

p5.js-web-editor/client/modules/IDE/actions/ide.js

Lines 94 to 98 in 7e069ee

	export function expandSidebar() {
	return {
	type: ActionTypes.EXPAND_SIDEBAR
	};
	}

But the action creators that are generated automatically by Redux Toolkit will always pass the argument as the payload property of the action.