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

REPL builder crashes on infinite loops #382

Open
brandonmcconnell opened this issue Sep 21, 2022 · 6 comments
Open

REPL builder crashes on infinite loops #382

brandonmcconnell opened this issue Sep 21, 2022 · 6 comments
Labels

Comments

@brandonmcconnell
Copy link
Contributor

brandonmcconnell commented Sep 21, 2022

Describe the bug

The REPL builder crashes on infinite loops rather than reporting them via an error and allowing the dev to modify/save the REPL to resolve the bug.

Oftentimes, if this happens in a REPL I'm working on, I have to continuously reload the page, quickly switch tabs, and copy the source to a local editor to resolve the bug, only to then paste the code back into a fresh REPL, as the infinite looping one is endlessly stuck.

Reproduction

I've created one such example that shouldn't crash immediately (on larger screens at least). However, if you shrink the viewport size of the REPL preview to where the text gets truncated, you'll see it freezes completely.

REPL: https://svelte.dev/repl/deaa7c99b06f4ecf8921174a88b33538?version=3.50.1

Logs

No response

System Info

Unnecessary — not local issue

Severity

annoyance

@Prinzhorn
Copy link

Prinzhorn commented Sep 21, 2022

This is the wrong repo for REPL, see https://github.com/sveltejs/sites/tree/master/packages/repl

I was about to comment the following on your original issue:

It'd be helpful if the REPL could somehow report that in an error and prevent the editor from crashing

Sounds like the halting problem. The REPL does prevent regular vanilla endless loops by injecting stuff (see loopGuardTimeout option). What you have here is impossible to detect in my opinion. It's an async/"recursiveish" endless loop (the browser will notify the observer in the next tick I would assume). But maybe some trickery is possible, but I don't see anyone invest considerable amounts of time in this.

However, one thing I would like see is the option to disable auto-eval in REPL. This would give us the chance to save before running it. Otherwise you can trigger endless loops just by typing code you didn't mean to run. I usually find it annoying, but I'm sure there's already an issue for that in the repo.

@benmccann can you please move this to the /sites repo?

@Conduitry Conduitry transferred this issue from sveltejs/svelte Sep 21, 2022
@Conduitry
Copy link
Member

Transferred. But I agree that this feature request certainly sounds halting-problem-y.

@benmccann benmccann added the REPL label Sep 21, 2022
@brandonmcconnell
Copy link
Contributor Author

@Prinzhorn Yeah, if there's some trickery we can do here to detect and prevent that, that'd be great.

I also agree— I think a way to set a REPL to be able to be saved first before running would be ideal, maybe as an option disabled by default.

@brandonmcconnell
Copy link
Contributor Author

Btw @Prinzhorn, here is another example that demonstrates the same issue, and I don't quite understand where the recursive issue would be coming from:
https://svelte.dev/repl/fa3352f06f004df4b0028f4fc654c8c0?version=3.50.1
☝🏼 un-comment the {(() => i++)()}

Here is the source, with that line pre-un-commented:

<!-- App.svelte -->

<script>
	let arr = [1, 2, 3];
	let i = 0;
</script>

{#each arr as elem}
	{(() => i++)()}
{/each}

☝🏼 this will break

What is it about running {(() => i++)()} within the template that causes such a break?

@brandonmcconnell
Copy link
Contributor Author

brandonmcconnell commented Sep 29, 2022

& related but without causing the loop issue— why, when I move the i++ logic into the script logic does it computed before the loop so all iterations have the same value:
https://svelte.dev/repl/a9c8110a13bb4ff88960850b96b2e2a4?version=3.50.1

Source:

<!-- App.svelte -->

<script>
	let arr = [1, 2, 3];
	let i = 0;
	let skip = (num = 1) => {
		i += num;
		return '';
	};
</script>

{#each arr as elem}
	{skip()} <!-- always renders 3 -->
	{i}
{/each}

@Prinzhorn
Copy link

Your last two examples are not directly related to this issue here. There are issues in Svelte about this already (sveltejs/svelte#7295 (comment)) and I personally think that your examples should straight up not compile in Svelte 4. I think at least some of the Svelte team was in the same boat as me on that. Mutating state from inside the template is undefined behavior and doesn't make sense. If you call a function it needs to be pure, e.g. sth. like formatDate(date). It should not have any side effects whatsoever.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants