T
TyphonJS•5mo ago
Vauxs

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
TyphonJS (Michael)
TyphonJS (Michael)•5mo 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
VauxsOP•5mo 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
VauxsOP•5mo ago
TyphonJS (Michael)
TyphonJS (Michael)•5mo 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
VauxsOP•5mo 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)
TyphonJS (Michael)•5mo 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
VauxsOP•5mo ago
No luck Not that it would work I think, the issue is when these subscription still exist
TyphonJS (Michael)
TyphonJS (Michael)•5mo 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
VauxsOP•5mo ago
would passing them as context instead of props do anything? 🤔
TyphonJS (Michael)
TyphonJS (Michael)•5mo 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
VauxsOP•5mo 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)
TyphonJS (Michael)•5mo 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
VauxsOP•5mo 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)
TyphonJS (Michael)•5mo 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.
Vauxs
VauxsOP•5mo ago
The item can be part of the same actor, but the bug also happens on WorldAnimations and that doesnt use TJSDocument at all Honestly just makes me ask what I did wrong when creating these windows because this seems like a basic issue to have in something like a system, where you can have multiple items open and you obviously dont want them to share state.
Vauxs
VauxsOP•5mo ago
No description
TyphonJS (Michael)
TyphonJS (Michael)•5mo ago
It's certainly possible that the problem lies outside of the Svelte components. Divide and conquer is the standard approach to take. Perhaps in the Svelte components for the actor / item variations explicitly retrieve an actor and item doc ala game.actors.get('<ID>') and game.items.get('<ID>') and see if there is sharing of data still. IE remove the external launching of the code and try to determine if the sharing lies with the TJSDocument / writable derived code.
Vauxs
VauxsOP•5mo ago
Made a test window and I managed to replicate this behavior once, but it then went away 😕 O Alright, I think I got a minimum repro, though it only happens on the first update Any subsequent update works normally aaaand it went away again
TyphonJS (Michael)
TyphonJS (Michael)•5mo ago
Well I appreciate your gumption as any info you glean will likely help out a 0.2.0 store based approach to get just a bit more out of TJSDocument in the interim before the Svelte 5 transition.
Vauxs
VauxsOP•5mo ago
Rewriting the whole approach, so far I don't see this bug returning Probably some weird interaction from my weird code. Now it's much nicer.
TyphonJS (Michael)
TyphonJS (Michael)•5mo ago
You can never rule out that six inches between ones ears! ;P Keep me posted. I found it curious that you saw similar issues between the demo code I posted and your effort.
Vauxs
VauxsOP•5mo ago
Finished the 2.0, it works perfectly lol
Vauxs
VauxsOP•5mo ago
GitHub
Windows 2.0 by MrVauxs · Pull Request #41 · MrVauxs/pf2e-graphics
Fixes #38 More generalized approach, more reliance on the svelte magic than writing your own stores. Also comes with a JSONEditor.
Vauxs
VauxsOP•5mo ago
I think the most meaningful change is the actual TJSDocument is now inside the AnimationEditor Which is shared across every window, as opposed to having the window making the TJSDoc and passing it as a prop
TyphonJS (Michael)
TyphonJS (Michael)•5mo ago
Just to point out the description above indicates a solution to the lapsed / dangling listener problem. As mentioned TJSDocument isn't as robust as it will be in 0.2.0, so I suspect that the issue you encountered likely is due to not manually destroying / cleaning up TJSDocument between app instances. I also haven't reviewed the whole scope of your code. When you switch wrapped core documents with a single instance of TJSDocument it will clean up the old associations with the core document when the switch is done. None of this is particularly a problem with TRL, but an age old anti-pattern common in all OOP languages / observer pattern. The nice thing though is that 0.2.0 will make this process a bit easier to get right. The improvement for 0.2.0 is that TJSDocument will only associate with the wrapped underlying core document when there are active subscribers to TJSDocument. If the subscriber count to TJSDocument drops to 0 then TJSDocument automatically unsubscribes to the core document. It will re-associate with the underlying core document if the subscriber count increases. With the current TJSDocument implementation there are two levels of indirection. 0.2.0 will make that one level of indirection where you just have to be concerned about managing the subsriber count directly with TJSDocument. When used in a Svelte component context this happens automatically.
Vauxs
VauxsOP•5mo ago
Feeling like printing this message out on paper and crushing it, its happening again :coolCry:
TyphonJS (Michael)
TyphonJS (Michael)•5mo ago
Well.. I'd be glad to take a look at the complete source code. You mentioned creating a small standalone instance of the problem right? Perhaps I can start there and take a look at causes then try it with the currently released TRL version and the improved TJSDocument support in the unreleased 0.2.0 version. If it goes away by just swapping that out then that is progress.
Vauxs
VauxsOP•5mo ago
I will try to replicate it but even as of now this is becoming tedious to find
TyphonJS (Michael)
TyphonJS (Michael)•5mo ago
Depending on the type of issue even reviewing the browser retained memory between snapshots in the dev tools can help identifiy a leak type problem which can occur with lapsed listeners. This is my assumption on what is going on though.
Vauxs
VauxsOP•5mo ago
I can no longer replicate the issue, however, instead I got a new one, the world settings aint getting updated grr Well I replaced storeSettings.getWritableStore with my own writable that on change just updates the setting. Will break with multiple GMs accessing the menu at once but beyond that... Not sure what is happening on the backend that TJSSettings is not saving the changes. I assume its to do with me storing an object of keyed arrays so the changes to the arrays are moot.
TyphonJS (Michael)
TyphonJS (Michael)•5mo ago
Ahhh... Yeah... This is something separate from the other TJSDocument issue. In the 0.2.0 TJSGameSettings you can set an object and it won't check / verify for equality. Alas the original TJSGameSettings code has a check which causes the object test to fail when setting an object. This has been long removed since Oct / Nov '23, but just hasn't been released.
Vauxs
VauxsOP•5mo ago
😩 FWIW both of these are in the same code and they do interact thru the titular bug (one window updates, somehow sometime the other window has that update too)
Vauxs
VauxsOP•5mo ago
No description
TyphonJS (Michael)
TyphonJS (Michael)•5mo ago
Well.. I'll do my best to wrap up current work and get an initial 0.2.0-next.1 / beta releases out. There will still be some rough areas in the standard library I'll be tweaking, but it's pretty important to get some of those long term committed fixes out.
Vauxs
VauxsOP•5mo ago
I'd gladly test it given the module is still in its relative infancy Backend is pretty much done, the only thing that gives me perpetual torment is the UI If it means that torment will be used for beta-testing so be it I need to wait for the animations library to get bigger anyway, I can't match my own autorec that quickly
TyphonJS (Michael)
TyphonJS (Michael)•5mo ago
This upcoming ~8 days is relatively free on my schedule, so I'm going to be smashing through as much as possible.
Want results from more Discord servers?
Add your server