ktmud opened a new pull request #11435: URL: https://github.com/apache/incubator-superset/pull/11435
### SUMMARY `src/explore/components/controls/withVerification` was introduced #7468 for a very specific use case here at Airbnb. It was not used anywhere within the official Superset. Since then, there were [some](https://github.com/apache/incubator-superset/pull/10264) [efforts](https://github.com/apache/incubator-superset/pull/10224) on improving the chart controls that have failed to consider this `withVerification` HOC: 1. Controls will now only re-render when their own values are updated. Previous a re-render may be triggered even if only other control values are changes. 2. Control configs now accept a custom React component as control type, meaning developers don't have to pre-register a control in `src/explore/component/controls/index.js`. This PR refactors the `withVerification` HOC to a functional component and renamed it to `withAsyncVerification`, with a couple of more enhancements and bugfixes: 1. Instead of setting `getEndpoint` that specifies a Superset API endpoint, developers can set an async `verify` function that returns an arbitrary promise. This makes it possible to run verification via other type of async operations---such as calling a typed API function created with [makeApi](https://github.com/apache-superset/superset-ui/pull/666). 2. It now accepts an `onChange` hook, which will be called every time users change control values. Developers have access to the full control props (as well as the the Redux actions by association) in the `onChange` callback, making it possible to trigger updates and revalidation on other controls. We could have removed `withVerification` from the Superset codebase for good since control configs now accept custom React components, but I figured this pattern might be useful to the community, too, so decided to keep it. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A ### TEST PLAN Unit tests. ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [x] Has associated issue: see links - [ ] Changes UI - [ ] Requires DB Migration. - [ ] Confirm DB Migration upgrade and downgrade tested. - [ ] Introduces new feature or API - [ ] Removes existing feature or API ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
