Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feat] Track unsaved files in REPL #6153

Merged
merged 1 commit into from
Dec 2, 2021

Conversation

joshnuss
Copy link
Contributor

Fixes sveltejs/svelte-repl#117

If a user has unsaved changes in the REPL and navigates away, all changes are lost. This has bit me quite a few times.

This PR adds a warning "There are unsaved changes" to prevent any the data loss.

This change is in conjunction with sveltejs/svelte-repl#156.

I'm opening this PR so that we can discuss if this method is correct or if we should a different path.

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with npm test and lint the project with npm run lint

@pngwn pngwn added site and removed site: REPL labels Jun 26, 2021
@benmccann benmccann changed the title Track unsaved files in REPL [feat] Track unsaved files in REPL Jun 30, 2021
@benmccann
Copy link
Member

The site was migrated to SvelteKit, so this would need to be rebased

I'm curious your thoughts between this and #6251 since it seems like we would only need one of the two functionalities

@bluwy
Copy link
Member

bluwy commented Nov 2, 2021

I think this feels better (and safer) than #6251. This would need a rebase though.

@tanhauhau
Copy link
Member

i've rebased the branch, looks good now. Need to publish a new version of the REPL after sveltejs/svelte-repl#156, before merging this branch

@benmccann
Copy link
Member

I pushed a new version with updated REPL, but the site won't load now. Hopefully Vite 2.7 will fix this or at least give a better error message. I can't easily test with pnpm overrides because I'm not sure how to update our crazy codemirror setup in svelte.config.js to be compatible with pnpm. Vite 2.7 should hopefully come out on Monday, then we can update SvelteKit and try this again

@benmccann
Copy link
Member

I tracked down the error to an issue in the REPL change: sveltejs/svelte-repl#188

@benmccann benmccann merged commit 6b48f0c into sveltejs:master Dec 2, 2021
@joshnuss joshnuss deleted the change-tracking branch December 2, 2021 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alert if repl hasn't been saved
5 participants