fix(nativeFilters): Speed up native filters by removing unnecessary rerenders

[Always-prog]

SUMMARY

Currently, rendering 20+ filters takes not normal lot of time due to two issues:

1. The issue with the Select filter. After rendering, Select Filter adds the __cache key to the dataMask, and changing dataMask triggering a redraw of all filters again. This happens for each Select filter in sequence. This can be resolved by adding __cache to the default dataMask.
2. The issue with the dataMaskSelected in the hooks is that it is specified as an object, which always triggers a redraw. This can be resolved by setting dataMaskSelected to JSON.stringify.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before

2023-09-13_12-13-34.mp4

After

2023-09-13_12-14-27.mp4

TESTING INSTRUCTIONS

1. Go to any dashboard
2. Add 30 select filters
3. Reload dashboard
4. See that filters take a lot of time for before render.
   5 (optional). You can add console log to FilterBarScrollContext such as console.log('rerendered filters bar!') to see the number of times it logs.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[michael-s-molina]

/testenv up

[github-actions]

@michael-s-molina Ephemeral environment spinning up at http://54.202.180.9:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

[kgabryje]

I love to see such perf improvements!
1 question about dataMaskSelected - do you think it's possible to make the object keep the same reference when there are no changes, so that the dependency array doesn't trigger an update unnecessarily without using JSON.stringify?
If not, I see that dataMaskSelected is in many more dependency arrays - do you think we should update them all?

[Always-prog]

@kgabryje Hi! I can't write code that keep reference if dataMask is no changed :P
And you're right, seems we don't need some JSON.stringify. I removed them.

[villebro]

One small cosmetic nit. I agree with @kgabryje , IMO the need for doing JSON.stringify on deps arrays is a symptom of something being done wrong upstream. But if it gets very complex, I'm ok with this, too

[villebro]

Oops

Suggested change

	console.log('rerender');
	console.log('rerender');

[michael-s-molina]

IMO the need for doing JSON.stringify on deps arrays is a symptom of something being done wrong upstream.

+1

[michael-s-molina]

I love the fix. It's a significant performance improvement but we need to make sure we're not breaking anything. Tagging @jinghua-qa and @sadpandajoe for awareness. I also tagged @justinpark as one of the reviewers.

[kgabryje]

To be fair, we do use quite a lot of JSON.stringify around the native filters already. I think that if this one's hard to avoid, we should just proceed and attempt to clean the whole native filters code in a larger refactor effort when possible

[michael-s-molina]

To be fair, we do use quite a lot of JSON.stringify around the native filters already. I think that if this one's hard to avoid, we should just proceed and attempt to clean the whole native filters code in a larger refactor effort when possible

Fair point. Let's try to remove them but that shouldn't be a blocker.

[Always-prog]

Hi!
Cypress has failed due to billing plan exceeded.

https://github.com/apache/superset/actions/runs/6288585657/job/17074191658?pr=25282

[kgabryje]

@Always-prog it’s been fixed already, I restarted the CI

[michael-s-molina]

Hi! Cypress has failed due to billing plan exceeded.

The Cypress error was fixed by #25399. You may need to rebase your PR if that fix is not included in your commit history.

[Always-prog]

@kgabryje Thank you!
@michael-s-molina CI tests has passed!

[justinpark]

@Always-prog
Please add the following fix on useFilterDependencies which misses shallowEqual on useSelector

superset/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/state.ts

Lines 33 to 35 in 0c083bd

	const dependencyIds = useSelector<any, string[] | undefined>(
	state => state.nativeFilters.filters[id]?.cascadeParentIds,
	);

import { shallowEqual, useSelector } from 'react-redux';
...
  const dependencyIds = useSelector<any, string[] | undefined>(
    state => state.nativeFilters.filters[id]?.cascadeParentIds,
    shallowEqual,
  );

[Always-prog]

@justinpark Added!

[justinpark]

LGTM

[github-actions]

Ephemeral environment shutdown and build artifacts deleted.