michael-s-molina commented on a change in pull request #17063:
URL: https://github.com/apache/superset/pull/17063#discussion_r727411306
##########
File path: superset-frontend/src/datasource/DatasourceEditor.jsx
##########
@@ -374,6 +374,40 @@ const defaultProps = {
onChange: () => {},
};
+function OwnersSelector({ datasource, onChange }) {
+ const selectedOwners = datasource.owners.map(owner => ({
+ value: owner.id,
+ label: `${owner.first_name} ${owner.last_name}`,
+ }));
+ const loadOptions = useMemo(() => (search = '', page, pageSize) => {
+ const query = rison.encode({ filter: search, page, page_size: pageSize });
+ return SupersetClient.get({
+ endpoint: `/api/v1/chart/related/owners?q=${query}`,
+ }).then(response => ({
+ data: response.json.result.map(item => ({
+ value: item.value,
+ label: item.text,
+ })),
+ totalCount: response.json.count,
+ }));
+ });
Review comment:
> I believe if you don't pass any dependencies to useMemo, it will still
recalculate on every rerender.
You're correct. If no array is provided, a new value will be computed on
every render. In this case, we are passing a function to the `options` prop of
the Select and this prop is used by `useEffect` hooks which means we don't want
to recompute it unless the dependencies change. We could use a `useCallback`
here but we also don't want to execute the function if it receives the same
parameters. That's why we generally use `useMemo` with a dependency array
(sometimes empty, sometimes not).
##########
File path: superset-frontend/src/datasource/DatasourceEditor.jsx
##########
@@ -374,6 +374,40 @@ const defaultProps = {
onChange: () => {},
};
+function OwnersSelector({ datasource, onChange }) {
+ const selectedOwners = datasource.owners.map(owner => ({
+ value: owner.id,
+ label: `${owner.first_name} ${owner.last_name}`,
+ }));
+ const loadOptions = useMemo(() => (search = '', page, pageSize) => {
+ const query = rison.encode({ filter: search, page, page_size: pageSize });
+ return SupersetClient.get({
+ endpoint: `/api/v1/chart/related/owners?q=${query}`,
+ }).then(response => ({
+ data: response.json.result.map(item => ({
+ value: item.value,
+ label: item.text,
+ })),
+ totalCount: response.json.count,
+ }));
+ });
Review comment:
Just to let you know, we are already working on the documentation for
each prop of the component to make it clear what are the different modes of
operation and options available. We're also going to update the Storybook with
these docs.
##########
File path: superset-frontend/src/datasource/DatasourceEditor.jsx
##########
@@ -374,6 +374,40 @@ const defaultProps = {
onChange: () => {},
};
+function OwnersSelector({ datasource, onChange }) {
+ const selectedOwners = datasource.owners.map(owner => ({
+ value: owner.id,
+ label: `${owner.first_name} ${owner.last_name}`,
+ }));
+ const loadOptions = useMemo(() => (search = '', page, pageSize) => {
+ const query = rison.encode({ filter: search, page, page_size: pageSize });
+ return SupersetClient.get({
+ endpoint: `/api/v1/chart/related/owners?q=${query}`,
+ }).then(response => ({
+ data: response.json.result.map(item => ({
+ value: item.value,
+ label: item.text,
+ })),
+ totalCount: response.json.count,
+ }));
+ });
Review comment:
Just to let you know, we are already working on the documentation for
each prop of the component and the different modes of operation. We're also
going to update the Storybook with these docs.
##########
File path: superset-frontend/src/datasource/DatasourceEditor.jsx
##########
@@ -374,12 +374,44 @@ const defaultProps = {
onChange: () => {},
};
+function OwnersSelector({ datasource, onChange }) {
+ const loadOptions = useCallback((search = '', page, pageSize) => {
+ const query = rison.encode({ filter: search, page, page_size: pageSize });
+ return SupersetClient.get({
+ endpoint: `/api/v1/dataset/related/owners?q=${query}`,
+ }).then(response => ({
+ data: response.json.result.map(item => ({
+ value: item.value,
+ label: item.text,
+ })),
+ totalCount: response.json.count,
+ }));
+ }, []);
Review comment:
Let's clarify the concepts a little bit
(https://reactjs.org/docs/hooks-reference.html)
- `useCallback` will return a memoized version of the callback that only
changes if one of the dependencies has changed. This is useful when passing
callbacks to optimized child components that rely on reference equality to
prevent unnecessary renders.
- `useMemo` will only recompute the memoized value when one of the
dependencies has changed. This optimization helps to avoid expensive
calculations on every render.
> Curious to what the error is when you just use the function directly?
If we pass the function directly, this means that every time the component
renders, it will create a new instance of the function, which will trigger the
`useEffects` inside the Select component, which will call the function and
execute it unnecessarily.
If we use `useCallback` it means that the function instance will be the same
between renders, avoiding unnecessary executions. But if while interacting with
the component, this function is called two or more times with the same
`search`, `page`, and `pageSize`, `useCallback` won't prevent unnecessary
executions. That's why we use `useMemo`. One example of this scenario is if we
change a property like `useFetchOnlyOnSearch` which will trigger a new call to
the function with the same parameters.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
