refactor: dead code cleanup: generated partial hooks - #318
Open
damyanpetev wants to merge 7 commits into
Open
Conversation
Every generated type overriding SerializeCore declared a `partial void SerializeCoreIgb<X>(RendererSerializer ser)` hook and called it right after `base.SerializeCore(ser)`. Repo-wide that is 141 declarations and 141 call sites with zero implementations -- dead weight on the per-render property serialization path, not just in the declaration list. In 22 types the hook call was the only statement in the override, leaving it forwarding to base and nothing else, so the override goes too. Those types have no serializable properties of their own; the only callers of SerializeCore are the virtual dispatch sites in BaseRendererControl and BaseRendererElement. Follow-up to the `OnCreated*` sweep (#316). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The inbound half of two-way binding called `OnEventUpdating<Prop>(this._field, ref newValue)` to let a partial adjust the value in flight before it was written back and `<Prop>Changed` was invoked. 16 declarations, 16 call sites, zero implementations, so the ref never changed and the call was a no-op on the value written back. Also drops the 16 stray empty statements the generator emitted directly above those call sites -- there are exactly 16 in src/, one per site, so none are left behind. The path is covered by the branch's binding contracts, which assert the inbound write-back for 19 .Bind pairs across 15 components. Follow-up to the `SerializeCoreIgb*` sweep and the `OnCreated*` sweep (#316). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The generator declared one of these beside every property backing field so a partial could rewrite the incoming value in the setter. 538 declarations, but only 46 call sites -- the call was emitted only for reference-typed properties (attached children, event-args details, RenderFragment templates), so 492 of the declarations were unreachable by construction rather than merely unimplemented. 45 of the 46 call sites dispatched to nothing. The 46th, IgbChat.OnOptionsChanging, had the only implementation in the family; its two statements now run inline at the same point in the Options setter, so the normalization is visible where it happens instead of in a partial two files away. With this the generated hook surface is down to the families that are actually used: FindByName* (5 implementations) and OnHandling* (2), plus GetSerializableTabsCollection. Follow-up to the `OnEventUpdating*` and `SerializeCoreIgb*` sweeps and to #316. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every generated type declared a `FindByName<X>(string name, ref object item)`
hook and an override that tried base, then the hook, then gave up. With the hook
unimplemented the override reduced to `return base.FindByName(name)`, so 135 of
them were forwarding-only and are gone entirely; `FindByName` stays reachable
through the virtual in BaseRendererControl/BaseRendererElement, unchanged for
callers.
The 5 implementations (Accordion, Dropdown, Select, TileManager, Tree) were five
copies of the same ContentItems scan sitting in componentsBase/WebInputs. Each
is now inlined into its own override, replacing the ref-parameter dance with a
direct return:
foreach (var item in ContentItems)
{
if (item.Name == name || item.ContainerId == name)
{
return item;
}
}
IgbTabs keeps its override for the ActualTabsCollection lookup, minus the hook.
Covered by the interop contracts: DropdownTests asserts a {"refType":"name"}
payload resolves to the .NET IgbDropdownItem instance through the inlined path,
and ExpansionPanelTests asserts the same for a self-reference through the base
implementation.
Follow-up to the `On<Prop>Changing`, `OnEventUpdating*` and `SerializeCoreIgb*`
sweeps and to #316.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
121 declarations, 121 call sites, 2 implementations. Each event setter wrapped
the hook in an onArgs lambda passed to SetHandler:
this.SetHandler<TArgs>(this.Name, "Opening", value, (args) =>
{
OnHandlingOpening(args);
});
104 of those lambdas contained nothing but the dead hook call. onArgs is an
optional parameter that SetHandler null-checks before invoking, so the argument
is dropped outright rather than left as an empty lambda -- one less closure
allocated per wired event. The other 15 lambdas carry two-way binding write-back
and keep their bodies, minus the hook call.
The 2 implementations become ordinary private methods named for what they do,
called from the same point:
IgbTabs.OnHandlingChange -> SyncSelectedTab
IgbInputBase.OnHandlingInputOcurred -> RaiseValueChanging
Both stay in componentsBase/WebInputs beside the EnsureXHandled helpers they
belong with; only the codegen-hook indirection is gone.
TabsTests asserts the selection sync through SyncSelectedTab. ValueChanging has
no test coverage, so RaiseValueChanging was kept a pure rename with an unchanged
body and call point.
Follow-up to the `FindByName<X>`, `On<Prop>Changing`, `OnEventUpdating*` and
`SerializeCoreIgb*` sweeps and to #316.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Clears the remainder of the hook surface, leaving zero `partial void` members in
src/.
IgbTab.OnIgbTabInitializing / OnIgbTabDisposing: declared and called in
OnInitializedAsync and Dispose, never implemented. Both gone.
IgbTabs.GetSerializableTabsCollection: declared and implemented in the same
generated file, so the partial bought nothing. Its body assigned
ActualTabsCollection unconditionally, which made the caller's read of
_tabsCollection dead:
{ var coll = this._tabsCollection; GetSerializableTabsCollection(ref coll); ser.AddCollectionProp("tabsCollection", coll); }
collapses to
{ ser.AddCollectionProp("tabsCollection", ActualTabsCollection); }
Verified on net8.0 and net10.0: 929 passed, 0 failed.
Follow-up to the `OnHandling<Event>`, `FindByName<X>`, `On<Prop>Changing`,
`OnEventUpdating*` and `SerializeCoreIgb*` sweeps and to #316.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes the generated
partial voidextensibility hooks fromsrc/components/Blazor/. They were a code-generation affordance: a seam for hand-written partials to intercept generated code. Now that the source is hand-maintained, the seam has no reason to exist — a hook with no implementation is unreachable code, and a hook with one is an indirection that reads better inlined at the call point.After this series there are zero
partial voidmembers insrc/. One commit per family so each can be reviewed on its own. Continues #316 (OnCreated*constructor hooks).SerializeCoreIgb<X>OnEventUpdating<Prop>On<Prop>ChangingFindByName<X>OnHandling<Event>OnIgbTab*,GetSerializableTabsCollectionCommits 1–6 total 149 files, +137 / -4830.
Commit 1 —
SerializeCoreIgb<X>Every generated type that overrides
SerializeCoredeclared a hook and called it as the first statement afterbase.SerializeCore(ser):141 declarations, 141 call sites, 0 implementations repo-wide, including
tests/andstories/. Unlike a bare declaration, which the compiler erases for free, each of these also occupied a line inside the per-render property snapshot path.partial void SerializeCoreIgb<X>(RendererSerializer ser);declarations removedSerializeCoreIgb<X>(ser);call sites removedSerializeCoreoverrides in classes with no declared serializable properties left with nothing to do, removed entirelyCommit 2 —
OnEventUpdating<Prop>The inbound half of two-way binding. When the client raises the change event for a bound property, the generated handler converts the payload, called this hook to let a partial adjust the value in flight, then wrote it back and invoked
<Prop>Changed:Also removed: the 16 stray empty statements (
;alone on a line)Commit 3 —
On<Prop>ChangingThe generator declared one of these next to every property backing field, and the setter called it so a partial could rewrite the incoming value before the property stored it:
partial void On<Prop>Changing(ref T newValue);declarations removedOn<Prop>Changing(ref value);call sites removedThe one implementation
IgbChat.OnOptionsChanginglived in src/componentsBase/WebInputs/Chat.cs and normalized the incoming options object. Moved into theOptionssetter it was called from, which is now self-explanatory:Commit 4 —
FindByName<X>Name-based ref resolution: the client sends
{"refType": "name", "id": "…"}andFindByNamemaps it back to the .NET instance. Every generated type declared a hook and an override that tried base, then the hook, then gave up:With the hook unimplemented that whole override reduces to
return base.FindByName(name), so 135 of the 141 were forwarding-only and are removed entirely.The 5 implementations (
Accordion,Dropdown,Select,TileManager,Tree) were five byte-identical copies of the sameContentItemsscan sitting incomponentsBase/WebInputs. Each is now inlined into its own override, and theref-parameter dance becomes a direct return:IgbTabsis the sixth case — it keeps its override for theActualTabsCollectionlookup, minus the hook.Commit 5 —
OnHandling<Event>Every event setter wrapped the hook in an
onArgslambda handed toSetHandler:104 of the 121 lambdas contained nothing but the dead call.
onArgsis an optional parameter thatSetHandlernull-checks before invoking (BaseRendererControl.cs:2952), so the argument is dropped outright rather than left as an empty lambda — one less closure allocated per wired event:The other 15 lambdas carry two-way binding write-back and keep their bodies, minus the hook call.
The 2 implementations become ordinary private methods named for what they do, called from the same point:
IgbTabs.OnHandlingChangeSyncSelectedTabIgbInputBase.OnHandlingInputOcurredRaiseValueChangingBoth stay in
componentsBase/WebInputsbeside theEnsureXHandledhelpers they belong with. Only the hook indirection is gone — the call site now names a real method instead of dispatching to a partial that may or may not exist.Commit 6 — the last three
IgbTab.OnIgbTabInitializing/OnIgbTabDisposing— declared and called inOnInitializedAsyncandDispose, never implemented.IgbTabs.GetSerializableTabsCollection— declared and implemented in the same generated file, so the partial bought nothing. Its body assignedActualTabsCollectionunconditionally, which made the caller's read of_tabsCollectiondead code:Found while sweeping, not fixed here
IgbTab.Dispose()adds the tab to its parent collection instead of removing it (src/components/Blazor/Tab.cs:59):OnInitializedAsyncimmediately above it does the identicalAdd. CompareIgbExpansionPanel.Dispose()in src/componentsBase/WebInputs/Accordion.cs, which callsContentItems.Remove(this). This looks like a copy-paste fault in the generator template — a disposed tab is re-added toContentTabsCollection, so the collection accumulates dead entries across re-renders. Left alone deliberately: it is a behaviour change that wants its own test and its own commit, not a line smuggled into a dead-code sweep.Edit: #319