TyphonJS•4w ago

Separate components unintentionally updating one another

I don't know how I ended up here, but I have a bizarre bug where I have two windows open and one of them sends data to another. There is no code that shares state between the two.

37 Replies

Vauxs•4w ago

https://github.com/MrVauxs/pf2e-graphics/blob/main/src/view/ItemAnimations/tabs/animation-config.svelte https://github.com/MrVauxs/pf2e-graphics/blob/main/src/view/ActorAnimations/submenus/actor-animations.svelte Item Animations sends data to ActorAnimations but not vice versa, nor did I build it to do that

TyphonJS (Michael)•4w ago

Hard to say from the two files posted. There could be problems with the writable derived usage. I don't actually use writableDerived and my custom stores use the simpler propertyStore helper which itself wraps writableDerived. So offhand without looking into things more I can't tell immediately if you have a problem there. It looks like that code is duplicated across files. Perhaps move it into it's own standalone utility function. The problem could be elsewhere. Offhand though probably not likely is reusing a single TJSDocument instance where the writable derived store is targeting the same TJSDocument instance across apps. I'm only mentioning that because the instance is a prop. Perhaps add console log statements into the writable derived code near setFlag and log the ID of the document being set to get a bit more insight into what is occurring.

Vauxs•4w ago

I have tested the same thing with your TJSDocumentProperty and getting the same results. What is weird is that one menu doesnt experience that issue while the rest do, even one thats not actually attached to a TJSDocument, but a setting. UserAnimationsShell has the exact same structure and yet isn't affected which is the strangest part for me Logging what documents are updated doesnt really help, I need to know what actually causes ItemAnimations to, somehow, send an update to ActorAnimations and make it update itself with that data And this only happens when the window is opened, so its definitely in the .svelte files themselves

Vauxs•4w ago

TyphonJS (Michael)•4w ago

So... I'd like to see how TJSDocument is handled presumably in the outer JS code. TJSDocument is leaky presently where the underlying handling doesn't manage registering with the Foundry doc efficiently. Manually invoking destroy on the TJSDocument instance might come into play when apps open / close. This will be fixed in 0.2.0 as TJSDocument will only associate with the underlying Foundry doc when there are actual subscribers to the TJSDocument instance.

Vauxs•4w ago

There isn't any outer code besides the creation of the window i.e. Button on Sheet passes Document to SvelteApplication -> SvelteApplication calls AppShell with the Document as a prop AppShell turns the prop (Document) into TJSDocument It then passes that TJSDocument to any child components, including adding new animations.

TyphonJS (Michael)•4w ago

Add an onDestroy handler where the TJSDocument instance is created. Something like: onDestroy(() => doc.destroy()); Just trying to rule out a deficiency / potential edge case here. As mentioned this will be resolved in 0.2.0 for more automatic handling.

Vauxs•4w ago

No luck Not that it would work I think, the issue is when these subscription still exist

TyphonJS (Michael)•4w ago

Yeah.. Probably not it, but good to keep in mind that the current TJSDocument is a bit weak in how it manages the connection to the underlying Foundry doc. It's always a good idea to manually destroy a TJSDocument instance when it goes out of scope for now.

Vauxs•4w ago

would passing them as context instead of props do anything? 🤔

TyphonJS (Michael)•4w ago

Just make the code cleaner with less prop drilling. You could have the TJSDocument instance in the outer SvelteApplication / JS code and invoke destroy on it when the app is closed for instance.

Vauxs•4w ago

That was my thought but then context is supposed to only work on the component it was created on and its descendants

TyphonJS (Michael)•4w ago

Here is my take on making a standalone writableDerived function; do modify it accordingly:

import { type Writable }      from 'svelte/store';
import { type TJSDocument }   from '#runtime/svelte/store/fvtt/document';
import { writableDerived }    from "#runtime/svelte/store/writable-derived";
import { isObject }           from '#runtime/util/object';

/**
 * Currently supports object data as the flag value.
 *
 * @param doc     TJSDocument instance.
 *
 * @param scope   The flag scope which namespaces the key.
 *
 * @param key     The flag key.
 *
 * @param [defaultValue] Optional default flag value.
 */
export function docFlagDerived<T>(doc: TJSDocument, scope: string, key: string, defaultValue: T = void 0): Writable<T>
{
   return writableDerived(
    doc,
    $doc => $doc.getFlag(scope, key) ?? defaultValue,
    (data, doc) => {
       function changeValue(data: unknown) {
          if (isObject(data)) {
             // iterating over the object using for..in
             for (const key in data) {
                if (data[key] === null) {
                   if (key.startsWith('-=')) {
                      delete data[key]
                      continue
                   }
                   data[`-=${key}`] = null
                   delete data[key]
                } else if (isObject(data[key])) {
                   changeValue(data[key])
                }
             }
          }
          return data
       }

       doc.setFlag(scope, key, changeValue(data))
       return doc
    },
   ) as Writable<T>;
}

import { type Writable }      from 'svelte/store';
import { type TJSDocument }   from '#runtime/svelte/store/fvtt/document';
import { writableDerived }    from "#runtime/svelte/store/writable-derived";
import { isObject }           from '#runtime/util/object';

/**
 * Currently supports object data as the flag value.
 *
 * @param doc     TJSDocument instance.
 *
 * @param scope   The flag scope which namespaces the key.
 *
 * @param key     The flag key.
 *
 * @param [defaultValue] Optional default flag value.
 */
export function docFlagDerived<T>(doc: TJSDocument, scope: string, key: string, defaultValue: T = void 0): Writable<T>
{
   return writableDerived(
    doc,
    $doc => $doc.getFlag(scope, key) ?? defaultValue,
    (data, doc) => {
       function changeValue(data: unknown) {
          if (isObject(data)) {
             // iterating over the object using for..in
             for (const key in data) {
                if (data[key] === null) {
                   if (key.startsWith('-=')) {
                      delete data[key]
                      continue
                   }
                   data[`-=${key}`] = null
                   delete data[key]
                } else if (isObject(data[key])) {
                   changeValue(data[key])
                }
             }
          }
          return data
       }

       doc.setFlag(scope, key, changeValue(data))
       return doc
    },
   ) as Writable<T>;
}

Vauxs•4w ago

Given the issue is that the TJSDocument is somehow sharing one-way updates to other unrelated components, maybe using contexts would get rid of the issue by enforcing that limit?

TyphonJS (Michael)•4w ago

Making sure that the TJSDocument instance is unique between both apps and making sure destroy is invoked on it should be the important part. Props vs context is more for code cleanliness. Also just a head check to make sure the actor and item are top level documents and say the item is not somehow and embedded item. Not saying that is the problem / more of a curiousity / edge case.

Gaming

Programming

Separate components unintentionally updating one another