Skip to content

Conversation

@andrewvarga
Copy link
Contributor

This is an alternative fix for #9186.

The previous fix used the current file's last modified date and compared it to when ZDS last updated the file to determine if changes on the file were caused by us or externally.

This alternative fix fixes the original issue by comparing the last written code to the file's content, instead of the current kclManager.code which was the main cause for the original bug.

There are 2 additional refinements:

  • show a toast when external changes have been detected to let the user know why their current content changed
  • only invoke resetCamera if we're in sketch mode. This way the most annoying part of the original bug, sketch mode should always be kept in orthographic view, even if there's a valid reload from disk.

@andrewvarga andrewvarga requested a review from a team as a code owner December 26, 2025 12:05
@vercel
Copy link

vercel bot commented Dec 26, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
modeling-app Ready Ready Preview, Comment Dec 30, 2025 8:09pm

// which would be evident if we already have matching code there.
if (!isCodeTheSame(code, kclManager.codeSignal.value)) {
const lastWrittenCode = kclManager.lastWrite?.code
if (!lastWrittenCode || !isCodeTheSame(lastWrittenCode, code)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the actual fix instead of using stat.mtimeMs


toast('Reloading file from disk')
// If we can use emojis this looks nice:
// toast('Reloading file from disk', { icon: '📁' })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks nice I just didn't want to start adding emojis to the codebase since we don't have any yet.

code: this.code ?? '',
time: Date.now(),
}
this.writeCausedByAppCheckedInFileTreeFileSystemWatcher = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved this down right before the actual write as I think that's technically the more correct way to do it.

Copy link
Contributor

@jtran jtran left a comment

Choose a reason for hiding this comment

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

I tried it out on sketch-solve and it seemed fine. Thanks for doing this!

@lee-at-zoo-corp lee-at-zoo-corp enabled auto-merge (squash) December 30, 2025 20:06
@lee-at-zoo-corp lee-at-zoo-corp merged commit a16221a into main Dec 30, 2025
54 of 56 checks passed
@lee-at-zoo-corp lee-at-zoo-corp deleted the andrewvarga/9186/camera-becomes-perspective-fix-cleanup branch December 30, 2025 20:07
franknoirot added a commit that referenced this pull request Jan 5, 2026
…le from disk" toast (#9516)

* Revert "Revert "Decouple CodeMirror from React" (#9515)"

This reverts commit 10bbff9.

* Forward fix for #9469 behavior: don't debounce kclManager.lastWrite

* Make the bot's bugfix recommendation
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.

4 participants