Skip to content

Track code changes#1368

Open
cocomarine wants to merge 3 commits intomainfrom
hj/track-code-change
Open

Track code changes#1368
cocomarine wants to merge 3 commits intomainfrom
hj/track-code-change

Conversation

@cocomarine
Copy link
Contributor

@cocomarine cocomarine commented Mar 6, 2026

Story: https://github.com/RaspberryPiFoundation/digital-code-club/issues/1056

Description

Now that caching is effectively disabled (#1342), we want to warn the users about losing their code changes upon locale change.
In CCP, we will be using this feature to trigger a modal to warn users and also allow them to confirm or cancel the locale change.

Changes

  • Added initialComponentContents to initial states and saved the initial contents on project load
  • Added codeHasChanged getter that compares any difference in components numbers and contents between initial and current projects

Copilot AI review requested due to automatic review settings March 6, 2026 15:34
@cocomarine cocomarine temporarily deployed to previews/1368/merge March 6, 2026 15:34 — with GitHub Actions Inactive
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds baseline tracking for project component contents so the web component can detect whether code differs from the initially loaded project state.

Changes:

  • Persist an initialComponentContents baseline in editor Redux state when projects are loaded or set.
  • Expose a codeHasChanged getter on WebComponent that compares current component contents against the stored baseline.
  • Initialize new editor state field for baseline tracking.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
src/web-component.js Adds codeHasChanged getter that compares current component contents to the stored baseline.
src/redux/reducers/loadProjectReducers.js Captures initialComponentContents during successful project load.
src/redux/EditorSlice.js Adds initialComponentContents to initial state and populates it in setProject.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines 203 to +207
setProject: (state, action) => {
state.project = action.payload;
state.initialComponentContents = (action.payload.components || []).map(
(component) => component.content,
);
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

initialComponentContents is used as the baseline for change detection, but it’s only set on project load (loadProjectFulfilled) and setProject. After a successful save/remix, this baseline isn’t updated, so any consumer of codeHasChanged will continue to report “changed” even when the current project has been persisted. Consider updating initialComponentContents in the saveProject/fulfilled and remixProject/fulfilled handlers (and any other action that should reset the baseline).

Copilot uses AI. Check for mistakes.
Comment on lines 124 to 126
runnerBeingLoaded: null | "pyodide" | "skulpt",
initialComponentContents: [],
};
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Storing full copies of every component’s content in initialComponentContents duplicates potentially large strings in Redux state (project contents are already stored under project.components). This can significantly increase memory usage and devtools serialization cost for large projects; consider storing a lightweight baseline instead (e.g., per-component hashes, a single combined hash, or a revision counter/timestamp that is updated when the baseline resets).

Copilot uses AI. Check for mistakes.
Comment on lines 14 to 18
state.project = action.payload.project;
state.initialComponentContents = (
action.payload.project.components || []
).map((component) => component.content);
state.loading = "success";
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic to derive initialComponentContents is duplicated here and in setProject (EditorSlice). Consider extracting a shared helper (e.g., getInitialComponentContents(project)) so the baseline computation stays consistent and future changes only need to be made in one place.

Copilot uses AI. Check for mistakes.
Comment on lines 203 to +207
setProject: (state, action) => {
state.project = action.payload;
state.initialComponentContents = (action.payload.components || []).map(
(component) => component.content,
);
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There’s no automated coverage verifying that initialComponentContents is initialized/reset correctly (and therefore that codeHasChanged can be relied on). Since this slice already has unit tests, please add tests for setProject and the load-project fulfilled path to assert that initialComponentContents matches the loaded project, and (if intended) resets after a successful save/remix.

Copilot uses AI. Check for mistakes.
@cocomarine cocomarine self-assigned this Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants