P5.js-web-editor: renaming files don't mark as saved

[shiffman]

• Found a bug
• tested chrome, mac os

Steps to reproduce:

1. Create a new sketch, add some code.
2. Save sketch.
3. Rename sketch.js to sketch1.js.
4. Change some code.
5. Save sketch.

After renaming the dot to mark a sketch as saved never goes away and a warning pops up if you attempt to leave or reload the page. However, as far as I can tell the sketch is actually saved (when I force reload my latest changes are there.)

[catarak]

thanks for submitting this!

[dhruvdutt]

@catarak I would love to try this. Can you please describe the fix?

[catarak]

i don't know off the top of my head how to fix this! there's a (redux) state variable that holds the state of whether or not the sketch needs to be saved. probably it's not getting set properly.

[himanshuc3]

@catarak Since the popup appears, it means the state variable probably would be affected. Can it be some change in scss files that is required?

[catarak]

@himanshuc3 i think it's more likely that the state isn't getting set properly—there's some logic that determines whether or not a project needs to be saved based on a sketch being changed by a user. i assume this logic is imperfect!

[awarua]

I was looking at it and didn't get very far, but for what it's worth, it looks like some of the relevant properties and functions might be unsavedChanges, setUnsavedChanges(), warnIfUnsavedChanges() and unsavedChangesDotUrl.

Hopefully, that saves someone else a little bit of time to find out what to search on.

[awarua]

I have been looking at this again. I believe that the piece of code that matters is /client/modules/IDE/actions/project.js#L87

if (!isEqual(currentState.files, response.data.files)) {
  savedProject.files = currentState.files;
  dispatch(setUnsavedChanges(true));
} else {
  dispatch(setUnsavedChanges(false));
}

I have stepped through the code in the debugger and when you are saving a project with a renamed file, it takes the first branch, so it seems that the currentState.files are not the same as the files that the response has...

Because of this, I checked that if any of the files has been renamed, the unsaved changes dot will not disappear – even if the file that is altered is not the one that was renamed.

[awarua]

The screenshot below shows the problem with the comparison on L87 above. There are two properties that are present in currentState.files that are not in response.data.files, which are isEditingName and isOptionsOpen. These probably get set on the local object because of the user interactions around renaming, but they are really not relevant to the intent of the comparison of the files.

[awarua]

I have created a pull request that fixes this problem by adding a custom comparison function rather than relying on the isEqual function from the lodash library. #652