Fix stale and clobbered state in Svelte applications

[jfn4th]

I was going through the Svelte build warnings and the state_referenced_locally ones led me to explore and experiment with how SvelteApplicationMixin hands state to its root component. Turns out the mixin has a couple of latent bugs affecting every Svelte-based app. This reworks how that state flow works.

TL;DR: SvelteApplicationMixin had two latent bugs: derivations reading stable references (Foundry documents) silently stopped updating on re-render, and user or peer state stored in the state object got clobbered every re-render. The visible result was trades that can't complete after any actor update. Fix: the state object is now strictly render-derived and replaced wholesale ($state.raw plus a getState() accessor); anything mutated from TypeScript moved to instance $state on the app (trade-dialog and compendium-browser became .svelte.ts). Measured performance is neutral.

The problems

The mixin kept one $state object and did Object.assign(this.$state, freshState) each render to merge in whatever _prepareContext returned. Two issues with that:

1. Reactivity that worked by accident. Object.assign onto a $state proxy skips writes where the value is the same reference (===). A lot of what _prepareContext returns is stable references, Foundry documents like actor.ancestry in the Attribute Builder for instance. Those fields never actually triggered reactivity on re-render. It only looked fine because derivations also read a fresh object (build, again on the Attribute Builder) that did trigger, and re-reading the mutated document while re-running gave the right answer anyway. Any derivation depending solely on something like data.ancestry.system.foo would silently go stale, with no warning.

2. State clobbering. _prepareContext returns the whole state object, including things like accepted: false, on the trade dialog. On re-render, Object.assign writes that straight over whatever the user had set. User and peer-driven state living in the state object got wiped every render.

In trade-dialog these combined into something genuinely broken. The component captured const { self, trader } = data once at mount (those state_referenced_locally warnings). After a re-render, $state.self was replaced with a fresh accepted: false object, but the component kept reading the old one. The checkmark stayed green while $state.self.accepted was actually false. UI and logic silently disagree.

In practice, any actor update after the dialog opens re-renders the dialog and breaks trade completion for one or both of the actors (which one depends on a few things, who had the actor update, who has already accepted, and the order in which events occur). Either the trade completes for one player and never completes on the other, or never completes for either even after both have accepted. The item doesn't transfer in either case. If neither actor is touched after the dialog opens it works fine. The bug needs at least one re-render.

Screen.Recording.2026-05-21.at.10.02.21.PM.mov

Compendium browser has a similar issue, but is dormant because it doesn't auto-re-render:

1. Open the browser, click into a tab
2. Console: game.pf2e.compendiumBrowser.render()
3. View snaps back to the landing page, active tab gets clobbered

The fix

The state object is now treated as strictly render-derived: replaced wholesale each render instead of merged, and typed $state.raw so the slot is reactive but the contents aren't. Components read it through a getState() accessor inside a $derived, so they always see the current object instead of a captured-once reference.

The key consequence of $state.raw: mutating this.$state.foo = bar no longer does anything reactive. That's deliberate, it makes the wrong thing fail loudly instead of silently. Anything actually mutated has to live elsewhere now.

So there are three clear homes for state:

• The state object: render-derived data, rebuilt by _prepareContext every render and never written afterward. The mixin injects a getState() accessor as a prop; components read it inside a $derived (const data = $derived(getState())) so they always pick up the freshly-replaced object instead of a captured-once reference.
• Component-local $state: UI-only state the component owns (search text, expand toggles), declared with $state in the component's <script>. Already worked, unchanged.
• Instance $state on the app class: state mutated from TypeScript that must survive re-renders. For example trade-dialog updating when the other player accepts or changes their offer, or compendium-browser's openTab() when an actor sheet or the sidebar tells the browser to jump to a tab. Declared as $state fields on the app itself, which is why those files have to become .svelte.ts. Components read it straight off the instance through the foundryApp prop (foundryApp.selfAccepted).

8 of 10 apps only needed the getState() swap; they were already just consuming render-derived data. Two needed instance state:

• trade-dialog: selfItems, traderItems, selfAccepted, traderAccepted are now $state fields on TradeDialog. The other player's trade updates (#onUpdate) and the component both mutate the same instance; _prepareContext can't touch them. It also overrides the mixin's $state accessor with a typed one, since #setGiftData still assigns this.$state directly and the override gets that write shape-checked (an app that never touches this.$state from TS, like compendium-browser, doesn't need this).
• compendium-browser: activeTabName and resultList moved to instance $state. openTab() (called from actor sheets and the sidebar) and the tab nav in the component both write activeTabName, so it can't live in the render-replaced state object.

Also cleaned up the state_referenced_locally and a11y warnings in the touched components. Svelte build warnings drop from 8 to 0 (surpressed an intentional one in filter-container).

Performance

Neutral to slightly positive. $state.raw skips the recursive proxying that plain $state does, so the render-derived state object is no longer wrapped in nested proxies; for the larger ones that's a small win on every access. Replacing the object wholesale is a single assignment instead of the old per-key Object.assign loop.

The one thing worth calling out: wholesale replacement invalidates every derivation reading the state each render. Before, only derivations reading a field that got a fresh reference re-ran; ones reading stable references (like data.ancestry) didn't, which is exactly problem 1. So "all derivations re-run now" mostly means the ones that were silently not updating finally do. The genuinely new cost is small: derivation re-runs are cheap, Svelte still diffs the DOM so a derivation that recomputes to the same result costs nothing downstream, and renders only happen when Foundry data actually changes, not on a loop.

I measured this on the attribute builder to be sure. With the builder open, 5 actor updates: render count was 5 before and after (we don't render more often); a derivation reading data.build re-ran 5 times before and after (no regression); a derivation reading only data.ancestry re-ran 0 times before and 5 times after. That 0-to-5 is the whole delta, and it's the bug being fixed: the derivation that was silently never updating now correctly does.

Most of this matters more later. The apps in this PR are small dialogs and pickers with modest state, so wholesale replacement is fine for them. Actor sheets are where it gets real: large nested state, more frequent re-renders, and _prepareContext already doing a lot of work. Regardless: the headline Svelte benefit, only the changed part of the sheet repainting instead of the whole thing, comes from Svelte's DOM diffing and is independent of the state model.

Fine-grained invalidation (for later)

The model in this PR is coarse: every derivation reading the state re-runs each render. Again, that's effectively free for these apps. Actor sheets will be bigger and re-render more often, so here are the options for going fine-grained and why none are worth doing yet, as reference for when it comes up.

First, why the old code couldn't be both. Foundry documents are passed by stable reference and mutated in place. Fine-grained invalidation needs a changed field to arrive as a fresh reference and an unchanged field to keep its old one. A Foundry document gives neither guarantee: the reference is the same whether or not its contents changed, so a === check can't tell the two apart. That's the root of problem 1.

There are two ways to fix that properly.

Route 1: deep-equal memoization. Have _prepareContext (or the mixin) deep-compare each field of the new state against the previous render's, keep the old reference where equal and use a fresh one where changed. A deep $state with per-field assignment would then invalidate correctly and finely. The problem is it trades one cost for another: instead of re-running the derivations, you deep-compare the whole state every render. On a large sheet that comparison costs about the same as the re-derivation it's trying to avoid, so it doesn't actually help.

Route 2: hook-driven granular state. Stop rebuilding state from _prepareContext on every render. Keep a long-lived reactive model on the app and patch only the affected $state fields in response to Foundry's updateActor and updateItem hooks, which carry a precise diff of what changed. Reactivity is then driven by what actually changed: no deep-compare, no coarse invalidation, and correct because each patch targets exactly the changed paths. This is genuinely fine-grained and correct. The cost is that it's a different data-flow architecture, not a mixin change.

On stores for route 2: Svelte 5 has largely superseded them. The official guidance is that shared reactive state should be a $state object in a .svelte.ts module (what the rest of this work uses), with stores kept mainly for complex async streams. So route 2's model should be $state, not stores. The one store feature that stays genuinely relevant is the start/stop notifier (writable(initial, (set) => { ...; return cleanup; })): it registers a subscription on first use and tears it down on last, a tidy way to bridge a Foundry hook into reactivity. That's a convenience rather than a capability, since the app already manages its own lifecycle, but worth knowing if the hook side ever gets genuinely complex.

Route 2 is the one worth doing eventually and route 1 isn't, the expensive part of a big sheet isn't derivation re-runs, it's _prepareContext itself (Foundry data prep, already heavy and made worse by the HP-change clone). Route 1 leaves that cost fully in place. Route 2 removes it for small changes, because patching one field skips the full re-prepare. So the real future optimization isn't "make derivations re-run less," it's "stop re-preparing the whole sheet for a one-field change," and route 2 is how you get there.

[CarlosFdez]

Before I finish reading, there is a lot, I remember @stwlam does not want documents at all to go in the state, and only wants plain objects for actual reactiveness. So it looks like that's something that slipped in. Wouldn't a document, which would be the same reference, not trigger reactivity?

[CarlosFdez]

Wouldn't const { self, trader } = $derived(data); fix those bugs without other changes? And not putting documents into state. If I recall the entire problem was that we did not use derived for anything coming from state, breaking the pipeline. I fixed it for some others, but I didn't adjust it for the trade window since that's stwlam's thing. What does using a getter to derive state from do over handling the warnings where they pop up?

[jfn4th]

Wasn't aware of not wanting documents in state. Attribute Builder (which I did) is the only one that currently has documents in state (ancestry, background, class), and that was before this PR. I can convert this to draft and mark it ready again after cleaning that up.

On the reactivity question specifically: with the old Object.assign merge a same-reference document genuinely didn't trigger anything. That's problem 1 in the description, with ancestry as the example. The new model replaces the whole state object each render, so derivations re-run regardless of a document inside being the same reference.

You're right that $derived plus keeping documents out of state fixes bug 1, and that's most of what the warnings pointed at. Let me lay out both options fully, since it's a real fork and the call is yours and Shark's.

The minimal version (no mixin change) requires three per-app disciplines:

1. $derived on every render-derived read. Mostly done already.
2. No documents in state; project to plain objects. Only attribute-builder violates this today.
3. Keep user/peer-mutated fields out of what _prepareContext returns.

Point 3 is what fixes bug 2 without touching the mixin. The clobbering isn't caused by Object.assign existing, it's caused by _prepareContext re-returning accepted: false so the merge overwrites the user's value. Under the current deep $state, a mutated field that simply isn't in _prepareContext's return is never touched by Object.assign: it persists in this.$state and stays reactive, because deep $state tracks in-place mutation. So compendium-browser could keep activeTabName in this.$state and mutate it directly, no clobber, no .svelte.ts rename. Same for trade-dialog's accepted.

So the minimal version is genuinely smaller: no mixin change, no getState, no file renames, just the per-app cleanup, as you mentioned.

What the mixin rework adds is robustness rather than correctness:

• $state.raw makes this.$state.foo = bar a reactivity no-op. That sounds like a downside, but it's the point: it makes "mutated state in the merged bag" fail loudly. Under the minimal version that pattern still works, so nothing stops bug 2 from returning the next time a field is added to _prepareContext.
• Wholesale replace makes "what's in state" stop mattering for correctness. A stray document, or a re-returned mutated field, degrades to coarse-but-correct instead of being a latent bug. For documents that latent bug has no warning: see legacyFlaws, correctly written with $derived, zero warnings, measured 0 re-runs across 5 renders on the old code.
• It splits the two kinds of state into named homes instead of one $state holding render-derived and persistent-mutable fields mixed together.

The tradeoff is correct-by-construction vs correct-by-discipline. The minimal version is correct as long as three unwritten rules hold forever, and two of them (no documents, mutated fields out of _prepareContext) have no warning to catch a violation. The rework removes the need for the discipline, at the cost of more .svelte.ts files, and the getState indirection.

I lean toward the rework, because the discipline has already slipped here: warnings sat unfixed, documents are in state, the trade bug is live. The rework doesn't enforce the three rules, it won't stop a document going into state, but it makes a slip contained rather than silent: a document in state becomes coarse-but-correct instead of silently stale, and $state.raw turns mutated-state-in-the-bag into a visible "UI doesn't update" instead of a clobber that ships. Under the minimal version every slip is a silent bug, and the no-documents one has no warning behind it. That's a judgment call on how much to trust the discipline going forward, and it's yours to make. Happy to scale back to the minimal version if you'd prefer.

[CarlosFdez]

On the reactivity question specifically: with the old Object.assign merge a same-reference document genuinely didn't trigger anything. That's problem 1 in the description, with ancestry as the example. The new model replaces the whole state object each render, so derivations re-run regardless of a document inside being the same reference.

I think our intent was that a re-render to a prop shouldn't occur if a value hasn't updated. The problem with the documents in state is that they are not reactive and thus we didn't want them in state. They're exclusively top level properties to invoke stuff on. However, if it turns out that states done our current way aren't reactive anyways, then perhaps a full sweep of all properties on the rerender, allowing documents to work, is a good idea, I just don't think that's the case?

I have not poked into this as much as you, so please understand that I'm trying to break down what you're saying because its a lot to absorb. It looks like the main benefit over using $derived is the case where someone writes to a state prop from svelte? So correct me if I'm wrong, so long as we use $derived, don't put documents in state, and somehow block writing to the state from the sheet, the bugs are resolved? Or is there something I'm missing.

I lean toward the rework, because the discipline has already slipped here: warnings sat unfixed, documents are in state, the trade bug is live.

If I remember right, the warning did not exist when these applications were written. But the other two (documents/should be read only) are valid points. We should find a way to enforce them.

Am I with you so far?

[jfn4th]

No worries! I understand it's a lot that I'm dropping here (my head has been swirling a bit going through all this).

Yeah, you've basically got it. A few things to tighten up.

The warning point is fair, if state_referenced_locally didn't exist when these were written then "warnings sat unfixed" isn't really a discipline slip. Scratch that one.

One thing on enforcement, since the rework only does half of what you'd want there. Read-only it does handle: with $state.raw the only thing that triggers reactivity is reassigning the whole state object, which the mixin does deliberately, once per render in _replaceHTML. A write into the current value (this.$state.foo = bar) still goes through as a plain JS write but triggers nothing, so a stray write just visibly does nothing instead of silently clobbering. No-documents it doesn't enforce though, and honestly that was never my goal, it doesn't stop a document going into state, it just keeps one from breaking things if it slips in (coarse-but-correct instead of silently stale). I'm still planning to keep state plain and do that cleanup on attribute builder when I can get back to this in the next few days, but that part stays a discipline. Hard-enforcing no-documents is trickier and I don't have a clean way to do it, so if you've got ideas there I'm all ears. I didn't know/start thinking about it until your first comment.

Your three-part summary is close. The only part I'd adjust is "block writing to state from the sheet", it's a bit too narrow, it only covers writes from the component. Those fields also get written from TS that isn't the sheet. trade-dialog's accepted from #onUpdate (the peer query handler), activeTabName from openTab() (which sheets and the sidebar call). So it's less "block sheet writes" and more "mutated state shouldn't live in the merged state object at all, no matter who writes it". Block only the sheet and the peer and external writers still clobber on the next render.

On "main benefit over using $derived": worth flagging that the rework still uses $derived too, components do const data = $derived(getState()) either way. So it isn't really rework-vs-$derived, $derived is on both sides. On its own $derived only covers the read-once-at-mount case, not documents or clobbering. The real difference between the rework and the minimal version is everything around it, how state flows in and where mutated state lives, not $derived itself.

On "are states done our current way reactive anyways", I think we actually agree: plain-object state is reactive, and documents aren't, which is the reason you already gave for keeping them out of state. The measurement just backs both up, ancestryBoosts (reads a plain object) re-ran 5/5 across 5 updates, legacyFlaws (reads only a document) re-ran 0/5.

The one nuance worth pulling out: that plain-object reactivity is already coarse. _prepareContext rebuilds every object and array fresh each render, so every object-prop derivation re-runs every render whether the content actually changed or not (ancestryBoosts was 5/5 on old code and 5/5 on new, identical). The "don't re-render if the value didn't update" intent really only holds for primitives right now. So the rework isn't giving up any fine-grained behavior for object props, there wasn't any there to give up.

And that coarseness is the same thing route 2 in the description is getting at. It comes from _prepareContext rebuilding everything wholesale, same root cause as the data-prep cost. Going hook-driven and granular later would clear both up, though the reason to actually do that is the data-prep cost on something like the character sheet, the derivation re-runs are cheap either way. As a small aside, $state.raw also skips the recursive proxying deep $state does, which could be a minor win once state gets large and nested, though I'm honestly not sure how much that'll matter in practice until we're in the character sheet.

I've got to get going for tonight, but we can keep discussing here or you can hit me up in the discord (@chromaticpenguin) if you'd prefer.

[CarlosFdez]

Sure. I'm poking you in part because I think stwlam was considering a rewrite of the mixin anyways, and having data on the table is useful even if we don't go with this solution.

On "are states done our current way reactive anyways", I think we actually agree: plain-object state is reactive, and documents aren't, which is the reason you already gave for keeping them out of state. The measurement just backs both up, ancestryBoosts (reads a plain object) re-ran 5/5 across 5 updates, legacyFlaws (reads only a document) re-ran 0/5.

The one nuance worth pulling out: that plain-object reactivity is already coarse. _prepareContext rebuilds every object and array fresh each render, so every object-prop derivation re-runs every render whether the content actually changed or not (ancestryBoosts was 5/5 on old code and 5/5 on new, identical). The "don't re-render if the value didn't update" intent really only holds for primitives right now. So the rework isn't giving up any fine-grained behavior for object props, there wasn't any there to give up.

So if I gather this right, my concern was that we're losing fine grained reactivity with this approach (for coarse instead), but you're saying that we don't have that for anything that isn't a primitive already. It looks like ideally then it'd be better to try to fix that so we do have fine grained reactivity rather than throwing it away so that documents work. That'll come in handy for sheets I think. If I remember right coarse grained reactivity is still better and faster than handlebars, but we might as well try to win it all if we can.

Hard-enforcing no-documents is trickier and I don't have a clean way to do it, so if you've got ideas there I'm all ears. I didn't know/start thinking about it until your first comment.

I don't either, its more that I wanted to frame the requirements for potential solutions. The only thing that comes to mind for "no writing into state from sheet" is readonly types or some sort of Object.freeze solution. I don't have a good idea for blocking documents with an error yet.

[jfn4th]

Yeah, makes sense. Happy for it to feed into whatever Shark does with the mixin even if this exact version isn't what lands.

Yes, object-prop reactivity is already coarse today, so the rework isn't giving up fine-grained behavior we had. Svelte tests object changes by reference, _prepareContext rebuilds every object fresh each render, so every object prop is a new reference every render and its derivations re-run whether or not the contents changed. Only primitives get value comparison, so only they are fine-grained today. I know I'm restating a lot of this, but just trying to be as clear as I can.

Now that I've slept on it, I'm not as confident in my hook based idea for fine-grained. But I do have some ideas for the readonly and document issues though. I'll have to dig into it a little bit more, hopefully work is slow today! Will report back, likely with more excruciating detail...

[CarlosFdez]

Looking forward to it! Some recursive approach might work for objects, but I have no clue for arrays of objects right now.

[jfn4th]

Okay, I ended digging into things way more than I expected (yes, work did end up slow yesterday lol)

TL;DR

• The hook approach doesn't hold up: it works off a source-data diff that can't be mapped to derived view-model slices through the rule cascade.
• Profiling: fine-grained would only optimize the minority share of per-update cost (roughly 8-22% depending on the character, on my end); the rest is prepareData, which is a separate question from sheet architecture.
• Memo is the best alternative I found, but it's manual dependency tracking, weakest exactly on the big sheets you'd want it for.
• So fine-grained isn't the blocker and shouldn't gate the bug fix; $state.raw types the coarse render-derived surface, and a fine-grained model would sit beside it, not replace it. Character-sheet perf is by and large a separate prepareData question.
• Coarse recompute isn't a real loss: Svelte diffs the result and touches only what changed, so the headline win over Handlebars' full subtree rebuild lands without any fine-grained machinery.
• Enforcement: the Readonly accessor type is the durable read-only guard; a dev-mode runtime check is IMO the pragmatic way to flag documents.

I've dug into the hook idea, it's shakier than I made route 2 sound. The sketch is the app holding a long-lived model and patching slices from updateActor/updateItem:

Hooks.on("updateActor", (actor, changes) => {
    if (actor !== this.actor) return;
    if (foundry.utils.hasProperty(changes, "system.build")) {
        this.model.build = prepareBuild(actor);
    }
});

The problem is changes is the source-data diff, the input to data prep, not the prepared output the sheet actually renders. One source write fans out unpredictably through the rule-element pipeline: a system.build change can re-evaluate GrantItem predicates, grant or remove items, and recalc modifiers, skills, HP. So there's no static map from the diff to a small set of view-model slices to patch. Patch too narrowly and you get stale UI, the exact bug class this PR fixes; patch broadly and there's no win.

And it's worth being precise about where the cost even is. actor.prepareData() (the rule pipeline) re-runs on every update regardless, the sheet can't skip it. The only thing a sheet controls is its own _prepareContext. So the win for any fine-grained scheme is skipping the view-model rebuild, not skipping data prep, which is smaller than route 2 stated.

I profiled it to be sure, console.timeing a few of the pregens at levels 1 and 5, plus a level 13 character I had access to and a quick in-combat scenario with a couple of party auras active on the whole group (I don't really have high level characters on hand right now). Reinitialize plus the full pipeline (actor.reset()) runs roughly 8-12 ms/update at level 1, 10-18 ms at level 5 (depending on character class and active effects), and ~22 ms at level 13. The current AppV1 sheet's getData runs roughly 1-4 ms across all of those. The transform's share of (prepareData + getData) is consistently a minority, running roughly 8-22% across the set: Kineticist's class layout pushes higher on the getData side, lighter level 5 characters run closer to ~15%, and the level 13 character drops to ~9% because reset grows faster than getData. Adding party auras pushed reset cost up on the affected characters but didn't move the share much, since active effects bulk up both sides of the split. prepareData dominates across the range, and heavier characters tilt further toward it, so you can't grow your way into fine-grained being worth it. On the character sheet, fine-grained reactivity optimizes that minority slice; the real cost is prepareData itself, which is system-core work, not a mixin or state-model concern.

The alternative that holds up better is per-slice input memoization: each view-model slice declares a small input footprint and rebuilds only when that footprint changed. Roughly, in _prepareContext:

const build = this.memo("build", actor.system.build.attributes, prepareBuild);
const skills = this.memo("skills", actor.system.skills, prepareSkills);

and memo itself is just a keyed cache:

#memo = new Map<string, { footprint: unknown; output: unknown }>();

protected memo<F, O>(key: string, footprint: F, rebuild: (footprint: F) => O): O {
    const cached = this.#memo.get(key);
    if (cached && fu.objectsEqual(cached.footprint, footprint)) {
        return cached.output as O;
    }
    const output = rebuild(footprint);
    this.#memo.set(key, { footprint, output });
    return output;
}

It deep-equals the footprint against last render and reuses the cached output when they match, so the prepare* call is the expensive part you skip. The footprint is prepared data, compared after the rule pipeline has already run, so cascades are handled by reacting to their result rather than predicting them, which is the thing the hook approach can't do. An update that only landed on build rebuilds that slice and leaves skills cached. It's route 1 from the PR description done surgically instead of deep-equal over the whole state.

The honest catch is this is $derived by hand. $derived auto-tracks a computation's dependencies for free, but only on reactive values, and Foundry's prepared data isn't reactive, so you hand-write the dependency list instead. On a big sheet that's a footprint per slice, each one a place to silently go stale if it misses an input. And it's upside-down: the cheap slices are cheap to rebuild anyway, while the expensive ones (attacks, spell lists, proficiencies) have large tangled footprints, exactly where the deep-equal stops being cheap and getting the footprint complete is hardest. So memo is correct where the hook isn't, but it's not a clean win for a full sheet; it's manual bookkeeping that's weakest on the slices you'd most want it for.

Net: fine-grained reactivity isn't the blocker here. For the 10 apps where there's no cost to remove, coarse is plainly fine. And by the profiling it isn't the lever for the character sheet either, since it optimizes the minority slice while prepareData sits untouched. So fine-grained isn't worth chasing right now and shouldn't hold up the bug fix; the real character-sheet question is whether prepareData is worth attacking, which is its own scope.

To be clear though, the headline Svelte win lands regardless of granularity. _prepareContext re-running wholesale is not the sheet repainting wholesale: Svelte diffs the recomputed result against the live DOM and only touches the nodes whose values actually changed. That's the real gain over Handlebars, which on every render serializes the whole sheet to a fresh HTML string, replaces innerHTML, and forces the browser to lay out and paint the entire subtree even when only one field moved. That render+paint cycle isn't inside the (prepareData + getData) split above (that's just data prep vs view-model transform); it's a separate slice on top of both, and on a big sheet it's a meaningful share of end-to-end per-update time. Coarse recompute plus fine-grained DOM updates is already most of the prize, and it falls out of the architecture without any per-slice memoization.

If _prepareContext ever does want trimming, the low-risk avenues are the ordinary ones: lazy per-tab context building (don't build the spell view model while you're on the skills tab), hoisting actor-independent work (config, localization) out of the per-render path, and debouncing the sheet's render to coalesce N updates in a tick (AppV2 serializes renders through Semaphore(1) but doesn't coalesce, so N updates in a tick is still N full renders). Per-tab is particularly attractive on Svelte because {#if activeTab === ...} skips both the derivations and the DOM for inactive tabs. All of these stay inside the minority share though, so the honest first move is still to profile prepareData and decide whether there's room.

On $state.raw and where it fits: it types one specific surface, the mixin's render-derived state object that _prepareContext rebuilds and _replaceHTML replaces wholesale. That surface is coarse by nature, it's rebuilt fresh every render so it can only be replaced, never patched, and $state.raw is the correct type for it permanently. It never becomes deep $state.

Fine-grained doesn't change that surface, it lives beside it. A fine-grained model is a long-lived deep $state object on the app instance, patched in place, which is already the third state home in this PR, the instance $state that trade-dialog uses. So a mixed future isn't a conflict: the mixin's state channel ($state.raw, coarse) and an app's instance $state (deep, fine-grained) are separate surfaces that coexist per-app. An app going fine-grained routes its data through instance $state and leaves the mixin's state channel for the genuinely coarse stuff, or empty. $state.raw on the mixin stays put either way.

On enforcement, your two requirements:

Read-only is the easy one, but worth being precise about what actually lasts. Right now $state.raw makes a stray this.$state.foo = bar inert at runtime, and you could Object.freeze each snapshot on top of that. But both of those lean on the state object being immutable, which only holds for a coarse, replace-wholesale surface. A fine-grained model has to be mutable since patching it is the whole point, so inert writes and Object.freeze both fall away there. The one read-only guard that carries across both surfaces is type-level: typing the component's accessor as Readonly<...> so a stray write doesn't even compile. So the $state.raw runtime inertness is really a bonus of the replace-wholesale surface, and the Readonly type is the durable piece, the thing to standardize on if read-only matters long term.

Blocking documents is harder. A type-level check would actually mostly work: apps hand-write precise state interfaces so a document in state almost always carries its real type, and a mapped type rejecting anything assignable to Document would catch it. The awkward part is where the constraint attaches. The mixin types state as just object, so enforcing it centrally means either each app opting its own state type in (convention, not enforcement) or threading the context type through the mixin as a generic and constraining it there, which is a fair bit of type machinery I haven't prototyped. The recursive type would likely be a bit heavy on the typechecker too until we're on tsgo, though this project has plenty of heavy types already. The pragmatic option might be a dev-mode runtime check instead: in _replaceHTML, shallow-scan result.state and warn on any value that's instanceof foundry.abstract.Document (or DataModel). Cheap, dev-only, fires right where the mistake gets introduced, and _replaceHTML is the one central spot to put it. It'd catch the attribute-builder case since those are top-level.

Some final thoughts since you raised winning it all. The three layers (core, system, mixin) aren't the same kind of fix.

Core: reactive data models would be the clean fix for the fine-grained side. If prepared data were reactive, $derived could track it directly and memo's hand-written footprints would just go away. Real architectural shift though so I wouldn't bet on it.

System: the majority slice (prepareData) is here, not core. Making it incremental would be considerable work and I won't guess at the shape, but drilling into where the time actually goes is the obvious first move. I did poke at that briefly. The two visible chunks were rule-element re-instantiation (every RE rebuilt from JSON through DataModel validation on each reset, which compounds because each rule's nested schema fields rebuild too) and the Statistic check/dc construction path (each Statistic lazily building both views, cloning all parent modifiers and re-extracting from synthetics each time). Between them they look like the bulk of prepareData. Both feel attackable short of a full incremental refactor. For the RE side, cache instances when their source rules haven't changed and reset only the per-prep mutable bits (predicates, ignored flags, label resolution) on each cycle, so DataModel validation only runs when the source actually changes. For the Statistic side, the check and dc sub-statistics each re-clone every parent modifier and re-extract from synthetics on construction; when there are no check-only or dc-only synthetics or inline modifiers (most skills most of the time), that's redundant and could be short-circuited to just adopt the parent's modifiers directly. Has anyone looked at either direction previously?

Mixin: however we reshape it, isn't where the cost lives. Worth saying plainly so the rewrite isn't expected to deliver perf it structurally can't.

[CarlosFdez]

I think I finally parsed this. prepareData() is known slow, and I've made attempts to speed it up, but I've noticed that a heavy chunk of that is unnecessary data model validations. Attempts otherwise have failed, even caching roll options doesn't seem to have helped much. A large chunk of what you're saying is about how slow prepareData() is. Statistic could be worth a go, it might be worth seeing if there is a case where the modifier clones could be skipped for performance for simpler statistics.

So lets say we give up fine grained, and its all coarse. Is it possible for documents to update without relying on a getState() function, aka with the current approach, or is it impossible for svelte to flag it as changed?

EDIT: For making it fine grained, I was really just considering a merge pass where we replace properties on existing objects recursively instead of Object.assign(). But that wouldn't solve arrays unless there is a consistent id or key prop on each object.

[stwlam]

Hey, break off this side conversation to somewhere if you intend for this to not be closed without merging. This isn't a discussion forum.

[jfn4th]

Pushed the attribute-builder cleanup, no more documents in state there.

On the getState in use here, yeah we could swap it for an actual getter on the mixin.

// mixin
get state(): object {
    return this.#state;
}

// component
const { foundryApp }: ... = $props();
const data = $derived(foundryApp.state);

@CarlosFdez I'll @ you on the discord when I'm looking into and messing around with merge pass/statistic optimizations/readonly/no documents in state enforcement.

[CarlosFdez]

If getState() flags all data as changed even if its just a single field with a document then there really isn't a reason to avoid documents there. I just think the getState() syntax is pretty weird and it'd be nice to have a different way of triggering a full refresh. If there isn't, then I'm alright with the getState() thing.

[jfn4th]

Agreed, the change to the attribute builder was more about consistency but I'll revert it.

Quick clarification because of how you phrased things. getState() doesn't trigger the refresh itself. The refresh is triggered by the mixin reassigning the $state.raw slot in _replaceHTML on re render. That reassignment is what fires reactivity for everything reading it. getState() is just the read path components use inside $derived to subscribe. So the syntax choice is about the read shape, not the trigger. The getter alternative I mentioned in my last comment (foundryApp.state) is the same mechanism with a different syntax. If the getter feels less weird I'll swap it.

[CarlosFdez]

Ah, I see. So the real thing you did was use $state.raw (which did not exist when the mixin was created) in order to trigger updates on every reassignment. Sure, that makes sense, I was operating under a fundamental misunderstanding the whole time. I'll think on the read shape independently once I circle back to svelte stuff, but I should have everything I need now, thank you.