TJSDocument discussion w/ VoodooFrog

Following up on this discussion from #dev-lounge from @VoodooFrog:

I have a TJSDocument for an actor in which I am creating an embedded store which filters out certain items on the actor. These are then each displayed with their own component which contains a select that sets some data. Now, I have created a new select control in the parent that sets all the items via an update to a given value, but upon doing so the child component is not reflecting the change in their own controls (and yes I am setting the selected value based on the data). Any ideas what might be happening or how I can fix it? I don't have the code in my repo yet, as I'm still working on it so I can't just link the source, unfortunately.

When you can commit something to take a look at that will help a lot.

17 Replies

VoodooFrog•9mo ago

https://gist.github.com/voodoofrog/357679ea18777710a18a2e8888b668ca I'll Gist it, so I don't have to commit yet. If you need to see the subcomponent, just ask and I'll put that up too. Well, actually - I think the one in the repo will be fine, I haven't made any changes to that. https://github.com/voodoofrog/vf-5e-sheet-addons/blob/master/src/components/SpellManagementComponent.svelte

TyphonJS (Michael)OP•9mo ago

I think the problem is that you are updating the embedded Item documents directly. The embedded collection is only reactive for changes of the collection itself and not the items / documents that it contains. It is not recommended to make TJSDocument instances for each spell / embedded collection document. What you can likely do is force an update to the embedded collection / DynMapReducer instance and this should cause the #each loop to run again: So this:

  const onChangeSelect = async (value) => {
    updating = true;
    for (const spell of spells) {
      await spell.update({ [`flags.${MODULE_ID}.${FLAGS.SPELL_SOURCE}`]: value });
    }
    updating = false;

    // This should work and force (true) an update to the reducer index and notify subscribers.
    // So the `#each` block should run again. 
    spells.index.update(true);
  };

  const onChangeSelect = async (value) => {
    updating = true;
    for (const spell of spells) {
      await spell.update({ [`flags.${MODULE_ID}.${FLAGS.SPELL_SOURCE}`]: value });
    }
    updating = false;

    // This should work and force (true) an update to the reducer index and notify subscribers.
    // So the `#each` block should run again. 
    spells.index.update(true);
  };

You might also be able to this with a reactive statement after redefining spells with let instead of const.

$: if (!updating) { spells = spells; }

$: if (!updating) { spells = spells; }

I'm only taking a stab in the dark here more or less. But it looks like you are updating the embedded documents, but the spells reactive embedded collection is only reactive for the collection and not the documents contained.

VoodooFrog•9mo ago

Yeah, I'm only wrapping TJSDocument around the actor. I figured doing so for each spell would be massive overkill. And I am indeed updating the items individually and not the collection, I figured there would be some way to get the collection to refresh after the updates, but that's where I got stuck. Hm... spells.index.update(true) still doesn't do it. 😦 I'll try redefining... ...still no. Hm. I feel like I'm missing something obvious.

TyphonJS (Michael)OP•9mo ago

You can put in a reactive log statement that should trigger when the embedded collection subscribers are notified:

$: console.log(`!!!! spells count: ${[...$spells].length}`).

$: console.log(`!!!! spells count: ${[...$spells].length}`).

Also make sure the previous reactive statement is triggering correctly by putting a console.log there.

$: if (!updating) { 
    console.log(`!!! Reassigning 'spells'`)
    spells = spells; 
}

$: if (!updating) { 
    console.log(`!!! Reassigning 'spells'`)
    spells = spells; 
}

VoodooFrog•9mo ago

I see both log statements on a change to the select. 🤷‍♂️ Have I screwed up my selected conditions in the subcomponent somehow? I know for sure that a hot-reload via code change switches them all correctly... but that doesn't really tell me much.

TyphonJS (Michael)OP•9mo ago

You can put in this statement in the #each block to have things log when the each block runs:

{#each [...$spells] as spell (spell.id)}
  {(console.log('#each block running'), '')}
  <SpellManagementComponent {spell} {classes} {mode} />
{/each}

{#each [...$spells] as spell (spell.id)}
  {(console.log('#each block running'), '')}
  <SpellManagementComponent {spell} {classes} {mode} />
{/each}

It will log repeatedly for the count of all the spells, but at least you can determine if the each block is running correctly then it very well could be a problem in SpellManagementComponent.'

VoodooFrog•9mo ago

(7) SpellbookManagerShell.svelte:48 !!!! spells count: 7
SpellbookManagerShell.svelte:44 !!! Reassigning 'spells'
SpellbookManagerShell.svelte:48 !!!! spells count: 7

(7) SpellbookManagerShell.svelte:48 !!!! spells count: 7
SpellbookManagerShell.svelte:44 !!! Reassigning 'spells'
SpellbookManagerShell.svelte:48 !!!! spells count: 7

It doesn't appear to have run. Although could that not also point to a problem with the each, like not being notified? I've also just realised if I wrap the each in an if using the updated var, then I can force it to re-evalute... but it looks a bit naff as it disappears for a moment before coming back.

TyphonJS (Michael)OP•9mo ago

Yeah.. A bit of a trick since the spells embedded collection is not changing. You can likely use a keyed block and that will work as intended without the if conditional.

{#key updating}
  {#each $spells as spell (spell.id)}
    <SpellManagementComponent {spell} {classes} {mode} />
  {/each}
{/key}

{#key updating}
  {#each $spells as spell (spell.id)}
    <SpellManagementComponent {spell} {classes} {mode} />
  {/each}
{/key}

You might have noticed dropping the [...$spells] array conversion. The latest Svelte should accept an iterator so you can use $spells in the #each block. Svelte does the array conversion internally; just syntactic sugar in this case.

VoodooFrog•9mo ago

Yeah, I can't get away with dropping the conversion. I'm on svelte 4.2.0. IIRC, I ran into dependency issues if I tried to go higher. But they key block has done the trick, thanks @TyphonJS (Michael) [UTC-7]! 🙂

TyphonJS (Michael)OP•9mo ago

I think in this particular case it's the best solution. We'll see what Svelte 5 can bring to the table. It is tricky to reactively monitor changes to documents in an embedded collection. The original #each block with the #key block will run if you added / removed spell items from the collection. I really wish the Foundry document model had a streamlined subscription mechanism vs the semi-heavyweight workaround w/ TJSDocument being required. TRL 0.2.0 does make TJSDocument more friendly / less error prone. One downside right now that is going away is that for the actorDoc you need to manually destroy that when the app shell closes via an onDestroy callback like:

import { onDestroy } from 'svelte';

const actorDoc = new TJSDocument(actor);

onDestroy(() => actorDoc.destroy());

import { onDestroy } from 'svelte';

const actorDoc = new TJSDocument(actor);

onDestroy(() => actorDoc.destroy());

In 0.2.0 TJSDocument is smart enough to connect to the Foundry document model only when there are subscriptions, so it will automatically clean up itself.

VoodooFrog•9mo ago

Thanks for all your help.

TyphonJS (Michael)OP•9mo ago

Yeah.. And you might just play around with using TJSDocument in SpellManagementComponent for the spell document. You just have to use the onDestroy callback for now otherwise things could get messy quick / dangling listener issue w/ the connection to the Foundry document. If the amount of spell items isn't expected to be super massive it should work OK.. Just going to be less error prone / less code in 0.2.0. A lot of this will evolve through the Svelte 5 transition.

VoodooFrog•9mo ago

Good to hear, I'll look forward to it. I'm pretty sure I had a TJSDoc for each spell in an earlier iteration, but like I said earlier I figured it was overkill.

TyphonJS (Michael)OP•9mo ago

It can be, but depending on the goal / complexity of the UI for the embedded documents could be handy. Better to wait until 0.2.0 for the peace of mind / not having to manually destroy TJSDocument instances.

VoodooFrog•9mo ago

Frankly, even with manual cleanup wrinkles, the current release of TRL is still miles better than dealing with handlebars.

El Saico•9mo ago

stumbles on the conversation This is what brought me to TRL; having liked Vue's reactive+component model, Svelte feels like the perfect alternative for a presentation layer library. Also, Tailwind (which became love at first try) suits reusable components much better.

VoodooFrog•9mo ago

I use Tailwind at work... haven't tried using it in a Foundry module yet. Mind you, the module discussed here is copying default 5e sheet styles anyway, so there'd not be much point in it. I do like Tailwind, though.

Gaming

Programming

TJSDocument discussion w/ VoodooFrog