ResizerService callback exception even with enableAutoResize=false

[jr01]

I'm submitting a Bug report

Your Environment

Software	Version(s)
Angular	12.2.6
Angular-Slickgrid	3.1.0
TypeScript	4.2.4

Describe the Bug

After upgrading to 3.10 one of our component UI tests consistently fails with:

Uncaught TypeError: Cannot read properties of null (reading 'hasOwnProperty')
    at TreeColumns.hasDepth (:9876/_karma_webpack_/webpack:/node_modules/slickgrid/slick.core.js:575)
    at applyColumnGroupHeaderWidths (:9876/_karma_webpack_/webpack:/node_modules/slickgrid/slick.grid.js:2701)
    at reRenderColumns (:9876/_karma_webpack_/webpack:/node_modules/slickgrid/slick.grid.js:2666)
    at legacyAutosizeColumns (:9876/_karma_webpack_/webpack:/node_modules/slickgrid/slick.grid.js:2661)
    at autosizeColumns (:9876/_karma_webpack_/webpack:/node_modules/slickgrid/slick.grid.js:2240)
    at SlickGrid.resizeCanvas (:9876/_karma_webpack_/webpack:/node_modules/slickgrid/slick.grid.js:3694)
    at ResizerService.resizeGridWithDimensions (:9876/_karma_webpack_/webpack:/node_modules/@slickgrid-universal/common/dist/esm/services/resizer.service.js:258)
    at ResizerService.resizeGridCallback (:9876/_karma_webpack_/webpack:/node_modules/@slickgrid-universal/common/dist/esm/services/resizer.service.js:227)
    at :9876/_karma_webpack_/webpack:/node_modules/@slickgrid-universal/common/dist/esm/services/resizer.service.js:218
    at timer (:9876/_karma_webpack_/webpack:/node_modules/zone.js/fesm2015/zone.js:2561)

Steps to Reproduce

This appears to happen when Angular SlickGrid is loaded and then gets unloaded/destroyed within 10ms. In our case a UI-test opens a page where Angular Slickgrid is shown and immediately navigates to another page.

I'm not sure why this occurs on 3.1.0 and didn't before, perhaps 3.1.0 initializes faster.

Expected Behavior

No exception should occur. And also I wouldn't expect this code to be executed at all since we're running with enableAutoResize=false.

Current Behavior

See above.

Possible Solution

The exception is caused by the 2nd resize here https://github.com/ghiscoding/slickgrid-universal/blob/master/packages/common/src/services/resizer.service.ts#L159:

	// -- do a 2nd resize with a slight delay (in ms) so that we resize after the grid render is done
    this.resizeGrid(10, newSizes);

which executes the resize in a setTimeout(..., 10). The callback doesn't check if Slickgrid still exists.

As mentioned I wouldn't expect this code to be executed because we set enableAutoResize=false.

For a fix, my thought is to add a condition in init here https://github.com/ghiscoding/slickgrid-universal/blob/master/packages/common/src/services/resizer.service.ts#L121:

if (this.gridOption && this.gridOptions.enableAutoResize) {
	this.bindAutoResizeDataGrid();
}

If you want I can create a PR on the universal repo for that. (PR 1)

I also noticed that when enableAutoResize=true the bindAutoResizeDataGrid gets called from bindResizeHook here: https://github.com/ghiscoding/Angular-Slickgrid/blob/master/src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts#L1049, so it appears in that case resize binding is done 2x and I think that is a separate bug and I can create a separate PR for that (PR 2).

I have found a workaround, but I should say first that we've been running without the Angular Slickgrid resizer for a while and are using a https://developer.mozilla.org/en-US/docs/Web/API/ResizeObserver because of a bug where the browser doesn't trigger the $(window).on(resize.grid${this.gridUidSelector}callback, but aResizeObserver` does trigger things.... I haven't gotten around to investigate that further. Anyway this is roughly the code & workaround we have in place now:

@Component({
	template: `
		<div style='height: 100%; width: 100%'>
			<angular-slickgrid
				[gridOptions]="gridOptions"
				(onAngularGridCreated)="onAngularGridCreated($any($event).detail)"
				...
		</div>
	`
})
public class MyGridComponent implements OnInit, OnDestroy {
	
	gridOptions!: GridOptions;
	
	private readonly _resizeObserver: ResizeObserver = new ResizeObserver((entries: ResizeObserverEntry[]) => this.doResize(entries));
	
	private _angularGrid!: AngularGridInstance;
		
	
	constructor(private _element: ElementRef) {
	}

	onAngularGridCreated(angularGrid: AngularGridInstance): void {
		this._angularGrid = angularGrid;
		
		// Workaround: disable the 2nd resizeGrid call by disabling the resizeGridCallback method.
		// Because the callback is executed from a setTimeout which hasn't executed yet this works.
		this._angularGrid.resizerService.resizeGridCallback = (_gridSizes?: GridSize): GridSize | undefined => _gridSizes;
		
		// Disable the Angular slickgrid resize and use a resizeobserver because the Angular slickgrid doesn't always detect to a resize.
		this._angularGrid.resizerService.pauseResizer(true);
		this._resizeObserver.observe(this._element);
	}

	ngOninit(): void {
		this.gridOptions = {
			enableAutoResize = false,
			...
		}
		...
	}
	
	ngOnDestroy(): void {
		this._resizeObserver.disconnect();
	}


	private doResize(entries: readonly ResizeObserverEntry[]): void {
		this.resizeGrid(entries[0].contentRect);
	}

	private resizeGrid(size: { height: number; width: number }): void {
		if (this._angularGrid && this._angularGrid.slickGrid) {
		  this._angularGrid.resizerService.resizeGridWithDimensions({
			height: size.height,
			width: size.width
		  });
		}
	}
}  

... and I noticed the resizeGridWhenStylingIsBrokenUntilCorrected workaround ... and I wanted to mention that we have a pretty similar scenario with a grid inside of a tab and user switching between tabs and resizing and I haven't seen that problem using the ResizeObserver approach...
so perhaps we could create a PR (PR 3) with a new option gridOptions.autoResizeOptions.useResizeObserver = true that when autoResizeEnabled=true would then use the ResizeObserver and no other workarounds.
With PR 1 + PR 2 + PR 3 I should then be able to completely remove any resizing workarounds from our code 😎

Code Sample

Sorry, I have no code sample. I can try to create one if needed of course.

[ghiscoding]

Oh ok that is a lot of info here, this extra resize with a 10ms delay was introduced to try to help/fix #820 (which I couldn't replicate so it's hard to fix that), the delay was in the previous version 2.x but it caused me problems on my project side (with the resizeGridWhenStylingIsBrokenUntilCorrected thing) and so I added 2 resize for that reason. The resizeGrid has to be called every time even with fixed height/width, it still has to be executed because these sizes are now read from the grid options then applied to the div container via that method, so the resizeGrid will do that and so it has to be executed even without the auto-resize (perhaps the method is confusing when used with static sizes).

As for the resizeGridWhenStylingIsBrokenUntilCorrected, it's an ugly hack and it's only in place for a nasty problem we have with our Salesforce implementation (where I spend most of my time nowadays with slickgrid) and that platform is kind of annoying because they have no Tab event that we can hook to when changing Tab and there's no way to bypass that either because of their use of shadow DOM and a component can't access anything outside its own container (shadow DOM) so I had to put that ugly piece of code in place just because I have no clue when the user changes tab (which is a seriously big joke, the concept is so simple yet it's not currently doable in Salesforce, which is so basically simple and that was what I used to do in Angular-Slickgrid).

The Resizer Observer seems interesting, I wonder if that would fix our problem, if you think it does then please go ahead with a PR. I also wanted to eventually get rid of the $(window).on and convert that Service to vanilla JS code (I'm slowly rewriting everything to vanilla, I'm currently in the process of moving all the plugins/controls code from 6pac/slickgrid into my lib and rewriting them in vanilla JS with full unit testing, it's an extremely long process but the goal is to remove jQuery code as much as possible and maybe one day...)

So yeah PRs are always welcome from you and I like your coding practices too, so totally open for PRs and discussions.

[ghiscoding]

@jr01 in regards to the PR you made in Slickgrid-Universal, would it be possible to add some details to describe the new option and how to use it in the Angular-Slickgrid - auto-resize Wiki? The Wikis are public, so you should be able to edit the Wiki straight away, it would be helpful for me and others to understand the changes/options.

Thanks again for the PR, I'm expecting a release tomorrow (Wednesday evening)

[jr01]

Yes I'll do that.

[ghiscoding]

@jr01
This is now released under version 3.2.0, check the Release v3.2.0 section for the entire explanation and remember to update both Angular-Slickgrid & all Slickgrid-Universal packages... thanks again for the contributions 🚀

[ghiscoding]

I see you updated the Wiki, thanks for that, I updated the Release with the new link.

Thanks for all the contributions you made, it's very much appreciated, it's rare to have good contributors like you and that helps on the motivation side to continue with the lib 😉