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

Current component should (probably?) not be available inside onMount #4259

Closed
Conduitry opened this issue Jan 14, 2020 · 6 comments · Fixed by #4909
Closed

Current component should (probably?) not be available inside onMount #4259

Conduitry opened this issue Jan 14, 2020 · 6 comments · Fixed by #4909
Assignees
Labels

Comments

@Conduitry
Copy link
Member

Describe the bug
Things that internally use get_current_component() should probably fail when they are called from an onMount callback.

Logs
None.

To Reproduce

<script>
	import { getContext, onMount } from 'svelte';
	import { get_current_component } from 'svelte/internal';
	onMount(() => {
		console.log(get_current_component());
	});
</script>

Expected behavior
I'd expect this to crash, but it doesn't. There is an exception if you use setTimeout instead of onMount.

Stacktraces
n/a

Information about your Svelte project:

  • Svelte 3.17.0

Severity
I'm not sure. I'm not actually positive this is a bug, but it feels like it. I don't think we want people to be able to call other lifecycle hooks or access the context within onMount.

Additional context
Came up in chat by way of a question about calling setContext in a component and then calling getContext during that component's onMount callback. It feels like this shouldn't actually be allowed.

@Conduitry Conduitry added bug awaiting submitter needs a reproduction, or clarification internals labels Jan 14, 2020
@dopry
Copy link

dopry commented Jan 14, 2020

I asked the original question on discord. The case I had was on https://github.com/dopry/svelte-auth0/blob/b11d46db898c2fea0015ae7ce0e56d7a6e9d1ab2/src/components/Auth0Context.svelte#L41

if I replace
const auth0 = await auth0Promise;
with
const auth0 = await getContext(AUTH0_CONTEXT_CLIENT_PROMISE);
auth0 is undefined.

You can test by running npm run showcase:dev

@Conduitry
Copy link
Member Author

Something else that just came up on chat is someone trying to call setContext() during a reactive code block. This won't provide the reactivity they were looking for - but also should this perhaps be a runtime error? Should the current_component also be unavailable during reactive declarations and blocks?

@Conduitry Conduitry removed the awaiting submitter needs a reproduction, or clarification label Feb 3, 2020
@Conduitry
Copy link
Member Author

Confirmed with Rich that this is not an intended feature, and that if it is easy to prevent this then we should.

@Conduitry Conduitry self-assigned this Feb 3, 2020
@dopry
Copy link

dopry commented Feb 3, 2020

Starting with documentation may be a good idea, if the compile time errors will take a moment to implement. My poor understanding is that OnMount is basically running at construction time and that the component isn't in a stable state for modification. And that rather than interacting directly with the component I should be interacting with in-scope variables that are used by the component instead.

@Conduitry
Copy link
Member Author

The above component now throws an error at runtime in 3.25.0.

@rmunn
Copy link
Contributor

rmunn commented Nov 15, 2021

Confirmed with Rich that this is not an intended feature, and that if it is easy to prevent this then we should.

@Conduitry - Do you remember this discussion from a year and a half ago? I ask because I'm trying to fix #6919 with #6920, and making the current component explicitly unavailable during onMount is what's causing #6919 (it makes bind:this not work during onMount).

Thing is, this is the example code from the bind:this docs:

<script>
	import { onMount } from 'svelte';

	let canvasElement;

	onMount(() => {
		const ctx = canvasElement.getContext('2d');
		drawStuff(ctx);
	});
</script>

<canvas bind:this={canvasElement}></canvas>

Those docs certainly imply that bind:this should work during onMount.

If you have any memory of the conversation with Rich that you mentioned on Feb 4, 2020 — or if you have any way to retrieve chat logs of that conversation from over 18 months ago — I'd love to understand the expected state of the component during onMount. Because right now, the Svelte docs seem to be promising something about bind:this that the library doesn't actually deliver, and I'd like to know whether the docs should be updated or whether I should pursue #6920 further.

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