MichaelKim / ag-grid-svelte

A Svelte wrapper for ag-grid
https://ag-grid-svelte.michael.kim/guide
MIT License
54 stars 15 forks source link

'Please don't call grid API functions on destroyed grids' error #8

Closed brunobely closed 1 year ago

brunobely commented 1 year ago
AG Grid: Grid API function setIsFullWidthRow() cannot be called as the grid has been destroyed.
Please don't call grid API functions on destroyed grids - as a matter of fact you shouldn't
be keeping the API reference, your application has a memory leak! Remove the API reference
when the grid is destroyed.

Sometimes when grids are destroyed and re-rendered I see this error thousands of times in the console, for many functions. I am binding the gridApi and columnApi properties but don't actually call them anywhere. I'm wondering if this is a known issue if there's a way around it?

MichaelKim commented 1 year ago

Could you give a code snippet example of this happening? I haven't seen this error before but I'll try reproducing locally.

brunobely commented 1 year ago

Yep! This is the smallest example I could think of (paste in a REPL and toggle the checkmark to reproduce):

<script>
  import AGGridSvelte from 'ag-grid-svelte';

  let api1;
  let api2;
  let checked;

  $: checked, api1?.setQuickFilter('');
  $: checked, api2?.setQuickFilter('');
</script>

<input bind:checked type="checkbox" />
{#if checked}
  <AGGridSvelte bind:api={api1} />
{:else}
  <AGGridSvelte bind:api={api2} />
{/if}

This REPL shows that setting api = null in onDestroy fixes the issue.

MichaelKim commented 1 year ago

Perfect, thanks for the example! Looks like when a grid gets mounted with the grid API of an unmounted grid, it will call the setters with the unmounted API before the grid is ready with the actual API. I'll have a fix out soon to address this.

MichaelKim commented 1 year ago

This should be fixed as of v0.2.1. The behaviour of the REPL should better match the React implementation; AG Grid only warns about calling setQuickFilter from an unmounted grid API rather than every setter.

brunobely commented 1 year ago

Makes sense, sounds like a good fix-- thank you for looking into it!