AG Grid upgrade to v34 #4062

lbwexler · 2025-08-06T04:05:17Z

Hoist P/R Checklist

Still pending a bit more testing, especially mobile, and also the tricky handling of pivot grid state in raw ag-Grid which had to change. (the latter is commented as being unreliable currently fwiw)

There is a branch agUpgrade that just has the changes to v32, but I don't think we are likely to use that -- we might as well go all the way to the current version, unless there is some unexpected problem with the new packaging.

Pull request authors: Review and check off the below. Items that do not apply can also be
checked off to indicate they have been considered. If unclear if a step is relevant, please leave
unchecked and note in comments.

Caught up with develop branch as of last change.
Added CHANGELOG entry, or determined not required.
Reviewed for breaking changes, added breaking-change label + CHANGELOG if so.
Updated doc comments / prop-types, or determined not required.
Reviewed and tested on Mobile, or determined not required.
Created Toolbox branch / PR, or determined not required.

If your change is still a WIP, please use the "Create draft pull request" option in the split
button below to indicate it is not ready yet for a final review.

Pull request reviewers: when merging this P/R, please consider using a squash commit to
collapse multiple intermediate commits into a single commit representing the overall feature
change. This helps keep the commit log clean and easy to scan across releases. PRs containing a
single commit should be rebased when possible.

lbwexler · 2025-08-07T21:19:03Z

New Sizes for ag-Grid, zipped:
./node_modules/ag-grid-community (350.93 KB)
./node_modules/ag-grid-enterprise/dist/package (307.28 KB)
./node_modules/ag-grid-react/dist/package (17.93 KB)
./node_modules/ag-charts-community/dist/package (316.13 KB)
./node_modules/ag-charts-core/dist/package (13.85 KB)

Prev sizes for ag-Grid, zipped. (react included in community)
./node_modules/@ag-grid-community (313.95 KB)
./node_modules/@ag-grid-enterprise (71.3 KB)
./node_modules/ag-charts-community/dist/package (194.4 KB)

@amcclain -- need to discuss
Substantial increase (~ .5MB to ~ 1MB). I think that's like a 10% increase in our overall zipped JS size (~5MB) but overall maybe not worth killing the upgrade for?
Is there something different we need to be doing with respect to tree shaking? Notice we are using "terser" tree-shaking but not "core" tree-shaking? (per comment in hoist-dev-utils below)

-Lee


// Disable package.json `sideEffects` based tree-shaking - was getting inconsistent
// results, with imports being dropped seemingly at random.

cr: Greg

amcclain

Awesome that we are off to the races on this. I took a first pass just now - need to break for a bit but will return to this testing more intensively with one or two selected apps.

amcclain · 2025-08-13T12:33:03Z

cmp/ag-grid/AgGridModel.ts

-                    // had ever been specified, `undefined` must be used.
-                    col.setSort(undefined, null);
-                    col.setSortIndex(undefined);
+                    agApi.applyColumnState({


Looks like the comment at the top of this conditional block is outdated and should be removed, is that correct? "ag-Grid does not allow "secondary" columns to be manipulated by applyColumnState..."

Now that we are able to use applyColumnState for both operations, should we be collecting all the changes - both for the singular AUTO_GROUP_COL_ID and any of these getPivotResultColumns() into one array of changes applied via single call to applyColumnState?

amcclain · 2025-08-13T12:35:23Z

cmp/grid/columns/Column.ts

-    ValueSetterParams
+    ValueSetterParams,
+    CustomCellEditorProps,
+    CellClickedEvent
 } from '@xh/hoist/kit/ag-grid';


Big 👍 on this change

amcclain · 2025-08-13T12:36:51Z

cmp/grid/impl/MenuSupport.ts

@@ -294,7 +295,7 @@ function levelExpandAction(gridModel: GridModel): RecordAction {
                return {
                    icon: isCurrLevel ? Icon.check() : null,
                    text: label,
-                    actionFn: () => gridModel.expandToLevel(idx)
+                    actionFn: () => wait().then(() => gridModel.expandToLevel(idx))


Is this a change we might make directly on current develop? I didn't test it out myself but I am guessing this gives us better menu dismiss behavior without a potential wait?

I'm always in favor of explicitly commenting waits and debounces so it's clear why we're doing them - otherwise they can be mysterious. Happy to add that assuming I'm correct about the reason behind the change.

amcclain · 2025-08-13T12:48:11Z

cmp/grid/Grid.ts

+                enableClickSelection: selModel.isEnabled,
+                isRowSelectable: () => selModel.isEnabled,
+                checkboxes: false,
+                headerCheckbox: false


We don't need to grow the scope of this upgrade - good arguments to be made for avoiding that! - but I'm struck that supporting checkboxes for selection is something that comes up from time-to-time and that we have a dedicated toolbox example around it. If we could pull through an option to use the built-in that might be a really nice win.

We could file as a follow-on ticket.

amcclain · 2025-08-13T12:53:21Z

cmp/grid/Grid.ts

            {contextMenu} = model;
        if (!contextMenu || XH.isMobileApp || model.isEditing) return null;

        // Manipulate selection if needed.
-        if (!agOptions.suppressRowClickSelection) {
+        if (model.selModel.isEnabled) {


Looks like a good change here, although makes me wonder why we were using the agOption before... 🤔 Do you suppose there was any chance we wanted/needed to ensure we respected the direct use of the option?

amcclain · 2025-08-13T13:18:13Z

cmp/grid/GridModel.ts

@@ -1562,7 +1563,7 @@ export class GridModel extends HoistModel {
                if (empty) {
                    agApi.showNoRowsOverlay();
                } else {
-                    agApi.hideOverlay();
+                    agApi.updateGridOptions({loading: false});


Noticed the asymmetry here where we don't unset loading in the empty case and clicked through to agApi.showNoRowsOverlay() impl - that method has a doc comment saying it will have no effect if loading. Tested and confirmed that the load mask does not re-appear - will see if I can adjust

amcclain · 2025-08-13T13:20:16Z

desktop/cmp/grid/editors/DateEditor.ts

-
+        const portalContainer = props.gridModel.viewRef.current?.querySelector(
+            '.ag-body-viewport'
+        ) as HTMLElement;


Worth making a getter on gridModel to encapsulate the selector and all that?

- `showNoRowsOverlay` documented as a no-op if `loading: true`

amcclain · 2025-08-13T13:23:42Z

cmp/grid/GridModel.ts

-                    agApi.hideOverlay();
-                }
+                agApi.updateGridOptions({loading: false});
+                if (empty) agApi.showNoRowsOverlay();


I made a change here but then tested a bit more and am not sure that we even need the call to showNoRowsOverlay - commenting it out, filtering a grid to no rows, then autosizing - I still see the overlay. Might be able to remove but didn't want to be overly hasty

# Conflicts: # CHANGELOG.md

lbwexler added 11 commits August 4, 2025 18:11

+ Checkpoint

78ea9d2

+ Checkpoint

30415fa

+ Checkpoint

2a4c343

+ Checkpoint

f2acc06

+ Checkpoint

b2919a9

+ Checkpoint

9151d8c

+ Checkpoint

dff9fe8

+ Checkpoint

3c457d1

+ Checkpoint

1353c6a

+ Checkpoint

3ad999a

+ Checkpoint

2e3b504

lbwexler requested review from TomTirapani and amcclain August 6, 2025 04:05

lbwexler changed the title ~~AG Grid upgrade34~~ AG Grid upgrade to v34 Aug 6, 2025

lbwexler closed this Aug 7, 2025

lbwexler reopened this Aug 7, 2025

lbwexler and others added 5 commits August 8, 2025 09:39

Merge branch 'develop' into agUpgrade34

7740d1a

Merge branch 'develop' into agUpgrade34

32ae96f

Fix to typing for ViewRowData

f86d8db

cr: Greg

Merge branch 'develop' into agUpgrade34

5110fb5

Move changelog entry to v76

6394b93

amcclain reviewed Aug 13, 2025

View reviewed changes

Fix to autosize masking + no rows overlay

214e2fe

- `showNoRowsOverlay` documented as a no-op if `loading: true`

amcclain reviewed Aug 13, 2025

View reviewed changes

amcclain and others added 4 commits August 14, 2025 08:50

Merge branch 'develop' into agUpgrade34

efd46b0

Merge branch 'develop' into agUpgrade34

246417d

# Conflicts: # CHANGELOG.md

Merge branch 'develop' into agUpgrade34

923049b

Suppress unwanted context menu

82ef81f

lbwexler removed the request for review from TomTirapani September 9, 2025 13:28

lbwexler requested a review from haynesjm42 September 9, 2025 13:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

AG Grid upgrade to v34 #4062

AG Grid upgrade to v34 #4062

Uh oh!

lbwexler commented Aug 6, 2025 •

edited

Loading

Uh oh!

lbwexler commented Aug 7, 2025 •

edited

Loading

Uh oh!

amcclain left a comment

Uh oh!

amcclain Aug 13, 2025

Uh oh!

amcclain Aug 13, 2025

Uh oh!

amcclain Aug 13, 2025

Uh oh!

amcclain Aug 13, 2025

Uh oh!

amcclain Aug 13, 2025

Uh oh!

amcclain Aug 13, 2025

Uh oh!

amcclain Aug 13, 2025

Uh oh!

amcclain Aug 13, 2025

Uh oh!

Uh oh!

AG Grid upgrade to v34 #4062

Are you sure you want to change the base?

AG Grid upgrade to v34 #4062

Uh oh!

Conversation

lbwexler commented Aug 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Hoist P/R Checklist

Uh oh!

lbwexler commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amcclain left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lbwexler commented Aug 6, 2025 •

edited

Loading

lbwexler commented Aug 7, 2025 •

edited

Loading