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

What would you say that $effect.tracking is for? #14329

Open
webJose opened this issue Nov 16, 2024 · 37 comments
Open

What would you say that $effect.tracking is for? #14329

webJose opened this issue Nov 16, 2024 · 37 comments

Comments

@webJose
Copy link
Contributor

webJose commented Nov 16, 2024

Describe the bug

I'm writing this as a bug, but perhaps it is not. Long story short: $effect.tracking() returns false during component initialization, but during component initialization, effect placement is valid. Up to 1 minute ago, I have always thought of $effect.tracking() as the one tool I need to know if an effect can be set up. This discovery has shaken this knowledge away from me.

If this is not a bug: Would you still say that $effect.tracking is the only tool at our disposal to safely set effects up? Because then how do we detect component initialization?

Reproduction

Not needed, I suppose?

Logs

No response

System Info

REPL

Severity

annoyance

@brunnerh
Copy link
Member

brunnerh commented Nov 16, 2024

It's there to detect whether the calling code expects reactivity or not and tracking whether any code is still actively "listening". This is mainly useful for code extracted to separate modules.

An example would be the implementation of a start/stop notifier as found on the readable store.

Example of an auto-updating Date state:

import { tick } from 'svelte';

let subscribers = 0;
let handle;
let value = $state(new Date());

export const date = {
  get current() {
    if ($effect.tracking() == false)
      return new Date();

    $effect(() => {
      if (subscribers++ === 0) {
        console.log('starting interval');
        handle = setInterval(() => value = new Date(), 1000);
        value = new Date();
      }

      return () => {
        tick().then(() => {
          if (--subscribers === 0) {
            console.log('stopping interval');
            clearInterval(handle)
            handle = null;
          }
        });
      };
    });

    return value;
  }
}

Playground

@webJose
Copy link
Contributor Author

webJose commented Nov 16, 2024

Thanks. What is your opinion about the fact that $effect.tracking() returns false when called from component initialization, then? It is a place where reactivity can take place, yet the function says it isn't. Bug or not?

@brunnerh
Copy link
Member

There is no tracking during component initialization, that would cause all sorts of problems, so this should be correct.
(There is a bug though, that $effect.tracking() returns false in the template.)

@webJose
Copy link
Contributor Author

webJose commented Nov 16, 2024

Ok, so is there a way to safely set effects up during component initialization? I mean, from non-component modules such as custom stores.

@brunnerh
Copy link
Member

I would say that is mostly an issue of intent and documentation.

Some effects are intended to live outside of component lifecycles, so they require their own $effect.root.
Other effects should be coupled to the calling component, in those, the calling code has to treat the function like an $effect call: It is only allowed to appear in places where $effect is allowed.

E.g. I also always document functions that internally use setContext/getContext with "has to be used during component initialization". Even if I could detect whether the code is running during initialization or not, there would not be any way to make the code behave in a sensible way: Without context access it would just be broken.

@webJose
Copy link
Contributor Author

webJose commented Nov 16, 2024

It is only allowed to appear in places where $effect is allowed.

Well, component initialization allows $effect. Yet, I have no way of knowing/asserting this. Or am I misunderstanding?

A custom module should be able to set an effect up:

class MyClass {
  #someState;
  constructor() {
    if (<something that can tell me I can set an effect up>) {
      $effect(() => ... );
    }
  }
}

@brunnerh
Copy link
Member

What would you do with that information?
Would your class still function as intended without the effect?

If you give a more concrete example of what you are trying to do, it might be easier to determine whether the API is missing something here.

@webJose
Copy link
Contributor Author

webJose commented Nov 16, 2024

Yes, for example, saving to local or session storage. Imagine the class needs to set the effect up for this. Also subscribing to the 'storage' event (and then unsubscribing). But the class could still function as a store without the effect.

@brunnerh
Copy link
Member

That seems like a case where you could also check $effect.tracking() only on access, rather than setting up an $effect immediately.

@webJose
Copy link
Contributor Author

webJose commented Nov 16, 2024

That seems like a case where you could also check $effect.tracking() only on access, rather than setting up an $effect immediately.

Can you elaborate on the reasons behind this?

@brunnerh
Copy link
Member

If the value is only accessed once, e.g. in a click event, you don't need an $effect to listen to events but can just get the current value.
If the value is added somewhere in the DOM, you can use the $effect to subscribe to the event and keep it updated & unsubscribe when it's no longer necessary.

If I understood what you are trying to do incorrectly, please give more detail.

@webJose
Copy link
Contributor Author

webJose commented Nov 16, 2024

Whether is accessed here or there, once or twice, should not matter to the conversation about the general availability of information, I think, but I suppose that yes, there are cases like that.

Still, moving the effect to the accessor is like 2 orders of magnitude more difficult than simply setting it up in the constructor. One has to count subscriptions and do getters and setters. All this can be avoided if there was something that could let me subscribe the effect on component initialization.

As it stands right now, it seems that the workaround (because it fells like one) is to go the complex route where one subscribes on value access.

@webJose
Copy link
Contributor Author

webJose commented Nov 16, 2024

Also, I just thought about performance: The subscribe-on-read approach sets one effect up per read operation. What if the value is read thousands of times? When I could have had just one effect during component initialization.

@webJose
Copy link
Contributor Author

webJose commented Nov 16, 2024

Look how simple things would be if Svelte gave the ability to set an effect up during component initialization:

// createStore.svelte.js
export function createStore(initialValue, componentInitializing) {
	const store = $state({ value: initialValue });
	if (componentInitializing || $effect.tracking()) {
		$effect(() => {
			console.log('Value: %o', store.value);
		});
	} 
	else {
		console.log('Not tracking.');
	}
	return store;
}
// App.svelte
<script>
	import { createStore } from './createStore.svelte.js'

	// Manually providing the missing part of the puzzle:  Because I, the dev, know this code is in
	// a script that initializes a component, I tell the function it is OK to set the effect up.
	let count = createStore(1, true);
</script>

This works. So my point here is: I think either $effect.tracking() should return true during component initialization, or there should be another API that accurately tells us whenever we can safely set effects up.

@webJose
Copy link
Contributor Author

webJose commented Nov 16, 2024

// createStore.svelte.js
export function createStore(initialValue, componentInitializing) {
	const store = $state({ value: initialValue });
	if (componentInitializing || $effect.tracking()) {
		$effect(() => {
			console.log('Value: %o', store.value);
		});
	} 
	else {
		// Maybe if the store cannot function without the effect, we can set up here with $effect.root.
		// Consumers would then need to know that there is some disposal needed to be done.
		$effect.root(...);
	}
	return store;
}

@trueadm
Copy link
Contributor

trueadm commented Nov 17, 2024

This works. So my point here is: I think either $effect.tracking() should return true during component initialization, or there should be another API that accurately tells us whenever we can safely set effects up.

We've discussed a $effect.active or some variation for this purpose but have waited for real-world use-cases.

@webJose
Copy link
Contributor Author

webJose commented Nov 17, 2024

That would be awesome. Can't wait!

@brunnerh
Copy link
Member

brunnerh commented Nov 17, 2024

I don't think this a confirmation that something will be added.

But if it is added, the name should be different, e.g. $effect.hasParent which would work well with $effect.root, i.e. if there is no parent you have to add a root first. (active is too similar to tracking and was its previous name after all.)

Also, as a workaround you can just use try/catch.

@webJose
Copy link
Contributor Author

webJose commented Nov 17, 2024

I know, hehe. It is dissimulated peer pressure. 😄

BTW, I was re-reading the docs on $effect.tracking. If this new API were to be equivalent to insideComponentInit || $effect.tracking(), would you say that $effect.tracking loses its purpose? Sounds like the new API, if it were a superset, can do the work of both.

Also, better $effect.hasRoot? Parent is kind of weird.

@brunnerh
Copy link
Member

$effect.hasRoot would work as well, the exact hierarchy should not matter.

$effect.tracking might still be necessary in certain scenarios, where you only want to perform actions in reactive contexts.

@webJose
Copy link
Contributor Author

webJose commented Nov 17, 2024

Maybe it is my ignorance, but I have only ever seen $effect.tracking being used to set effects up. Component initialization works for that too, so it is hard for me to imagine any other use of this rune that would require exclusion of component initialization time.

@paoloricciuti
Copy link
Member

I think a comment from Rich in the pr really nails down the usage of $effect.tracking: is not to know if you can setup an effect, is to know if that piece of code if being tracked or not. The simplest thing that you can do to create an effect or not is use a try catch around it.

But as @brunnerh was saying is much better to create the effect on read instead of effect while also keeping track of the number of effects created so that you only create the effects once.

@webJose
Copy link
Contributor Author

webJose commented Nov 17, 2024

Hello, @paoloricciuti and thanks for dropping by with the definition Rich gave upon creation of the rune.

As simple as it might be, I don't condone the practice of driving logic by exception. Sure, I'll do it if I must, but I never encourage it, so the simplest thing in your eyes, for me, is the very last resort.

As for your statement "is much better to create the effect on read instead": How is that better than not having to track subscriptions at all and simply allow the setup in component initialization, where you don't have to track anything? I don't get that part. If you can elaborate, I'll be grateful.

@paoloricciuti
Copy link
Member

If you create the effect in the constructor you might create the effect even when you don't need to: if the value is never read from a tracking context there's no need to create the effect and creating it in the constructor will even disallow people from using your utility outside a component. Sure you might wrap the effect in $effect.root or even skip the effect creation all together but the constructor will always run once per creation.

If you move the creation logic to the read (guarded by $effect.tracking and count for the amount of effect created) you can just read the value normally if you are not in a tracking context and if you are in a tracking context you can setup the effect that will make sure if the value is updated from outside the new value will be available in the UI. Notice this is only necessary if you read from a tracking context (which is what Rich comment was about).

An example of this is the MediaQuery utility from runed

export class MediaQuery {
    #propQuery: MaybeGetter<string>;
    #query = $derived.by(() => extract(this.#propQuery));
    #mediaQueryList = $derived(browser ? window.matchMedia(this.#query) : null);
    #effectRegistered = 0;
    #matches: boolean | undefined = $state();

    constructor(query: MaybeGetter<string>) {
        this.#propQuery = query;
    }

    get matches(): boolean | undefined {
        // here we register the effect only if we are in a tracking context
        // and no effect has been registered yet
        if ($effect.tracking() && this.#effectRegistered === 0) {
            $effect(() => {
                this.#effectRegistered++;

                useEventListener(
                    () => this.#mediaQueryList,
                    "change",
                    (changed) => (this.#matches = changed.matches)
                );

                return () => {
                    this.#effectRegistered--;
                    this.#matches = undefined;
                };
            });
        }
        // we check if we already have some state that has been set by the effect otherwise
        // we read directly from `#mediaQueryList`...you can use this in or out of components
        return this.#matches ?? this.#mediaQueryList?.matches;
    }
}

@webJose
Copy link
Contributor Author

webJose commented Nov 17, 2024

I understand all that. But doesn't that have a bug? What about this logic?

  1. ComponentA reads matches. This installs the effect in the context of ComponentA.
  2. ComponentB reads matches. This doesn't install an effect.
  3. ComponentA unloads. This triggers effect cleanup.
  4. ComponentB is still alive, but with an unreactive matches property.

As far as I can tell, the creation of one effect per read is unavoidable. It is the only way to keep up properly with the count. Or am I missing something here?

@paoloricciuti
Copy link
Member

It should work because on cleanup you also set matches to undefined retriggering the read from ComponentB and re-registering a new effect.

Another solution could be to always register an effect on the first read to keep track of the subscribers (but only on the first read, not on every read) and deregister the listener only if subscribers drop to 0 (basically reimplementing the start stop notifier)

@webJose
Copy link
Contributor Author

webJose commented Nov 17, 2024

and deregister the listener only if subscribers drop to 0

This is the part that is impossible to do unless you install one effect per read. How else can you safely decrease the number of subscribers?

@webJose
Copy link
Contributor Author

webJose commented Nov 17, 2024

It should work because on cleanup you also set matches to undefined retriggering the read from ComponentB and re-registering a new effect.

Ok, for this particular case maybe it works. In general, the pattern doesn't seem to hold for any and every possibility.

@paoloricciuti
Copy link
Member

and deregister the listener only if subscribers drop to 0

This is the part that is impossible to do unless you install one effect per read. How else can you safely decrease the number of subscribers?

It's not one effect per read, but one effect per subscriber. It's very different if you read that value a lot.

@paoloricciuti
Copy link
Member

It should work because on cleanup you also set matches to undefined retriggering the read from ComponentB and re-registering a new effect.

Ok, for this particular case maybe it works. In general, the pattern doesn't seem to hold for any and every possibility.

Can you show me some case where it doesn't?

@webJose
Copy link
Contributor Author

webJose commented Nov 17, 2024

It's not one effect per read, but one effect per subscriber. It's very different if you read that value a lot.

How does one identify subscribers? How can I know if 2 reads are from the same subscriber?

@webJose
Copy link
Contributor Author

webJose commented Nov 17, 2024

Can you show me some case where it doesn't?

Any case where cleaning up cannot be reversed, or is costly to set up again? I don't have one in mind right now. I'm trying to keep my mind in the "all-purpose" realm, as Svelte aims to be an all-purpose framework.

I don't know, say, you lose on cleanup a large amount of data, because cleaning up makes sense for garbage collection. Re-aqcuiring the data would be costly.

@paoloricciuti
Copy link
Member

It's not one effect per read, but one effect per subscriber. It's very different if you read that value a lot.

How does one identify subscribers? How can I know if 2 reads are from the same subscriber?

Oh yeah I think you are right on this. But I still think it's better than creating an effect in the constructor unless you know you will be use that utility only inside components.

Can you show me some case where it doesn't?

Any case where cleaning up cannot be reversed, or is costly to set up again? I don't have one in mind right now. I'm trying to keep my mind in the "all-purpose" realm, as Svelte aims to be an all-purpose framework.

I don't know, say, you lose on cleanup a large amount of data, because cleaning up makes sense for garbage collection. Re-aqcuiring the data would be costly.

Yeah but we need to talk about concrete cases. You could always keep the data and reset a separate version state that you also read inside the getter (to retrigger the read and thus creating a new effect) for example

@webJose
Copy link
Contributor Author

webJose commented Nov 17, 2024

Yeah but we need to talk about concrete cases. You could always keep the data and reset a separate version state that you also read inside the getter (to retrigger the read and thus creating a new effect) for example

Then the large amount of data is never cleaned up, "just in case". There will never be certainty as to when to clean it up.

But I still think it's better than creating an effect in the constructor unless you know you will be use that utility only inside components.

I was confronted with this before. There are 2 possibilities:

  1. The class/data structure can work without an effect installed.
  2. The class/data structure cannot work without an effect installed.

If the $effect.hasRoot rune existed (or maybe $effect.canTrack?), I could do:

if ($effect.canTrack()) {
  $effect(...);
}
else {
  // This part would be if the thing cannot work without the effect.
  this.#cleanup = $effect.root(() => {
    $effect(...);
  });
}

@paoloricciuti
Copy link
Member

Then the large amount of data is never cleaned up, "just in case". There will never be certainty as to when to clean it up.

Again that's why you need to go case per case. Is a huge amount of data that you want to garbage collect as soon as possible? Go with the effect per read approach. Is some simple data that can stay in memory? Go with the version approach. Is something that can be easily cleaned up and restored? Set to undefined. You know you just need it in a component? Try catch the effect creation and create a root (which is basically what you are asking). You can't have a catch all solution of the domain is so vast but you need to determine what the best solution for your use case is.

@paoloricciuti
Copy link
Member

Also the solution with effect.root has the disadvantage that you need to find a way to clean your root up.

@webJose
Copy link
Contributor Author

webJose commented Nov 17, 2024

I understand all you say, so I guess we are at an impasse, or at least I think we reached the end of the discussion. I'll summarize my position for the core team's final consideration.

  1. Using try..catch instead of branching is bad practice. In most runtimes, unwinding the call stack is a costly operation and the main reason why this is bad practice.
  2. I understand that using $effect.root() carries an extra step: Figure out cleanup. I know this, and the core team knew this when the rune was created and made public. Nothing new here.
  3. As I regard the solutions provided by master programmer @paoloricciuti the whole thing became complex really fast. This is not feeling sveltey (simple, beautiful, magical): A lot of possibilities to take into account every time the pattern comes up for consideration. This is dangerous grounds, in my humble opinion.
  4. If a value is read thousands of times (say inside an {#each}), thousands of effects will be registered when doing the effect-per-read strategy where proper tracking is needed (because cleanup is critical to happen at the exact time, and things cannot be restarted easily).
  5. A good approach for some cases could be to set up a single effect during construction, and for this a new rune is needed.

I think that summarizes my position. I thank you all for the discussion. I leave this topic in your capable hands.

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

No branches or pull requests

4 participants