Skip to content

refactor: allow grid header and footer renderers to run multiple times#9757

Open
vursen wants to merge 3 commits into
mainfrom
grid-rerunnable-header-footer-renderers
Open

refactor: allow grid header and footer renderers to run multiple times#9757
vursen wants to merge 3 commits into
mainfrom
grid-rerunnable-header-footer-renderers

Conversation

@vursen

@vursen vursen commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

The grid web component is moving to rendering its header and footer declaratively with Lit (vaadin/web-components#12134). With that change, header and footer cells can be re-created, which re-runs the column renderers. The connector wrapped these renderers in singleTimeRenderer, so a re-created cell would stay empty.

The sorter element is now cached between renderer runs: re-creating it on every run would reset the sort direction and send a needless sorters-changed event to the server.

Part of vaadin/web-components#10789

@vursen vursen changed the title refactor: allow grid header and footer renderers to run multiple times [WIP] refactor: allow grid header and footer renderers to run multiple times Jul 17, 2026

@vaadin-review-bot vaadin-review-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the changes — left 1 comment.

Finding
👀 The new "renderers run multiple times" behavior and the vaadin-grid-sorter caching have no test.

@vursen

vursen commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

@claude Investigate why single time renderers were added in the first place.

@claude

claude Bot commented Jul 17, 2026

Copy link
Copy Markdown

Claude finished @vursen's task in 2m 34s —— View job


Why singleTimeRenderer was added

I traced the connector's git history. singleTimeRenderer was introduced in #4025refactor!: remove template usage from grid header and footer (Sascha Ißbrücker, Nov 2022), commit d790e87b.

What that PR did

It replaced the old Polymer <template>-based header/footer rendering with the grid web component's imperative renderer API (column.headerRenderer / column.footerRenderer). As part of that, the header/footer content stopped being an HTML template string and became a live DOM node owned by FlowheaderComponent.getElement() (or a plain text string) — passed straight to the connector:

// AbstractColumn.java (introduced in #4025)
grid.getElement().executeJs(
    "this.$connector.setHeaderRenderer($0, { content: $1, showSorter: $2, sorterPath: $3 })",
    this.getElement(), headerContent, showSorter, sorterPath);

The connector renderer, back then, also cleared the cell on every run:

column.headerRenderer = singleTimeRenderer((root) => {
  root.innerHTML = '';            // wipe cell
  const sorter = document.createElement('vaadin-grid-sorter');  // fresh sorter each run
  ...
});

Why it was wrapped in singleTimeRenderer

With the imperative renderer API, the grid web component may invoke a cell's renderer more than once during that cell's lifetime. Letting it re-run was undesirable because each run would:

  1. Recreate the vaadin-grid-sorter — discarding the current sort direction and firing a needless sorters-changed round-trip to the server.
  2. Clear and re-append a Flow-managed node (root.innerHTML = '' then re-append content). Flow owns that node's lifecycle independently, so having the web component repeatedly wipe and rebuild the cell was pointless churn and risked fighting Flow's own DOM management.

Since header/footer content only legitimately changes when Flow calls setHeaderRenderer/setFooterRenderer again — which installs a fresh renderer closure — rendering once per closure was the simplest safe behavior. singleTimeRenderer enforced exactly that: run the wrapped renderer on the first cell paint, then no-op.

Why this PR reverses it

The web component is moving to declarative Lit-based header/footer rendering (vaadin/web-components#12134), where header/footer cells can be re-created. A re-created cell needs its renderer to run again to be populated — with singleTimeRenderer, re-created cells would stay empty. So the constraint that made single-time rendering safe no longer holds.

The one original benefit worth keeping — not recreating the sorter — is preserved by caching it in the closure instead:

let sorter;
column.headerRenderer = (root) => {
  if (showSorter) {
    sorter ??= document.createElement('vaadin-grid-sorter');  // reused across runs
    ...
  }
};

So the current PR keeps the sort-direction / no-extra-round-trip guarantee that motivated the original code, while dropping the "render once" restriction that the new declarative rendering makes incorrect.

Note (unrelated to your question): the review bot's point still stands — there's no WTR test exercising a renderer running twice with a re-created root, so a regression to const sorter = ... or to a single-run guard would ship undetected.
· branch grid-rerunnable-header-footer-renderers

The grid web component is moving to rendering its header and footer declaratively with Lit (vaadin/web-components#12134). With that change, header and footer cells can be re-created, which re-runs the column renderers. Rendering only once would leave re-created cells empty. The sorter element is cached between runs because re-creating it would reset the sort direction and notify the server.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vursen
vursen force-pushed the grid-rerunnable-header-footer-renderers branch from db7cac1 to 69a4349 Compare July 20, 2026 06:16
Covers moving component content into a column group after the grid has rendered, which re-runs the header and footer renderers on re-created cells.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vursen vursen changed the title [WIP] refactor: allow grid header and footer renderers to run multiple times refactor: allow grid header and footer renderers to run multiple times Jul 20, 2026
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants