"File > New" needs to be clicked twice to create a new sketch

[lindapaiste]

I have noticed an issue in the editor recently where sometimes I have to click File > New twice in order to trigger the action. I wondering if others have also been experiencing this, and if what @eyaler is trying to suggest could be related to that?

Originally posted by @raclim in #2541 (comment)

[lindapaiste]

After the first click on "File > New" I see the "Opened New Sketch" toast and the URL changes, but the sketch files and the text in the editor are still showing the previous sketch. Clicking "File > New" a second time works.

My guess would be a problem in the Redux code, or possibly a missing useEffect dependency somewhere.

[lindapaiste]

Relevant code:

p5.js-web-editor/client/modules/IDE/hooks/useSketchActions.js

Lines 24 to 32 in 031c3d0

	function newSketch() {
	if (!unsavedChanges) {
	dispatch(showToast('Toast.OpenedNewSketch'));
	dispatch(newProject());
	} else if (window.confirm(t('Nav.WarningUnsavedChanges'))) {
	dispatch(showToast('Toast.OpenedNewSketch'));
	dispatch(newProject());
	}
	}

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

Lines 272 to 277 in 031c3d0

	export function newProject() {
	setTimeout(() => {
	browserHistory.push('/');
	}, 0);
	return resetProject();
	}

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

Lines 266 to 270 in 031c3d0

	export function resetProject() {
	return {
	type: ActionTypes.RESET_PROJECT
	};
	}

p5.js-web-editor/client/modules/IDE/reducers/project.js

Lines 38 to 39 in 031c3d0

	case ActionTypes.RESET_PROJECT:
	return initialState();

After looking at this, it seems like the URL change to / is interfering with the intended change in the Redux state. Either by preventing the intended action (less likely), or by overwriting it with some actions triggered by the new page (more likely).

[kirtanchandak]

can I try this one out?

[lindapaiste]

can I try this one out?

@kirtanchandak i assigned this one to myself because I think it’s a bit complex and probably not a beginner-friendly issue. And also because I’ve previously worked on this part of the code and I might be responsible for it not working right now 😳. But if you want to try it then feel free to do so! It will give you an opportunity to understand some parts of the codebase, whether or not you are able to solve it. Look into the hook in the IDEView where we initiate the getProject fetch.

[lindapaiste]

@kirtanchandak I’m thinking about this more and I think I know what I did that broke it. I will explain more later but you should try removing the setTimeout in the redux action and see if that fixes it.

[lindapaiste]

Here's a lengthy explanation of what happened and how the setTimeout is involved.

This issue is my fault, and happened when converting the IDEView from a class component to a function component. We have a code in the IDEView which fetches the project from the API based on the id in the URL.

Here's that code before:

p5.js-web-editor/client/modules/IDE/pages/IDEView.jsx

Lines 112 to 118 in 314e4d0

	componentWillUpdate(nextProps) {
	if (nextProps.params.project_id && !this.props.params.project_id) {
	if (nextProps.params.project_id !== nextProps.project.id) {
	this.props.getProject(nextProps.params.project_id);
	}
	}
	}

And after:

p5.js-web-editor/client/modules/IDE/pages/IDEView.jsx

Lines 98 to 103 in f99eda0

	useEffect(() => {
	const { project_id: id, username } = params;
	if (id && project.id !== id) {
	dispatch(getProject(id, username));
	}
	}, [dispatch, params, project.id]);

You can see that there are some conditions which got removed. I recall discussing this with @dewanshDT and saying that some of those conditions didn't make sense. Now I understand that they were there for this specific scenario of navigating from a saved project to the root URL.

The current (bad) logic is "fetch the project if there is an id in the URL, and the id in the URL it doesn't match the id of the current project".

The previous logic was "fetch the project if there is an id in the URL, and there was not an id in the URL on the previous render, and the id in the URL doesn't match the id of the current project". Which is kind of weird logic, right?

We are definitely setting a new project but then overwriting it back it the previous project. And that's happening because the URL change is in a setTimeout. Which means that there is a single render where the project in the Redux state is the new project but the id in the URL is for the old project. That will trigger the useEffect to fetch the old project.

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

Lines 272 to 277 in 031c3d0

	export function newProject() {
	setTimeout(() => {
	browserHistory.push('/');
	}, 0);
	return resetProject();
	}

If we want the new logic to work, then all we need to do it delete the setTimeout and make it so that the URL change and the Redux state change happen on the same render.

[mhsh312]

This is why switching from legacy components is scary tbh

[raclim]

@mhsh312 it could be daunting but it's definitely a necessary change 😂

@lindapaiste Thanks so much for confirming and tracing back this issue!