bito-code-review[bot] commented on code in PR #39460:
URL: https://github.com/apache/superset/pull/39460#discussion_r3278075907
##########
superset-frontend/src/dashboard/components/SliceAdder.tsx:
##########
@@ -174,295 +164,284 @@ function getFilteredSortedSlices(
.filter(createFilter(searchTerm, KEYS_TO_FILTERS))
.sort(sortByComparator(sortBy));
}
-class SliceAdder extends Component<SliceAdderProps, SliceAdderState> {
- private slicesRequest?: AbortController | Promise<void>;
-
- static defaultProps = {
- selectedSliceIds: [],
- editMode: false,
- errorMessage: '',
- };
-
- constructor(props: SliceAdderProps) {
- super(props);
- this.state = {
- filteredSlices: [],
- searchTerm: '',
- sortBy: DEFAULT_SORT_KEY,
- selectedSliceIdsSet: new Set(props.selectedSliceIds),
- showOnlyMyCharts: getItem(
- LocalStorageKeys.DashboardEditorShowOnlyMyCharts,
- true,
- ),
- };
- this.rowRenderer = this.rowRenderer.bind(this);
- this.searchUpdated = this.searchUpdated.bind(this);
- this.handleSelect = this.handleSelect.bind(this);
- this.userIdForFetch = this.userIdForFetch.bind(this);
- this.onShowOnlyMyCharts = this.onShowOnlyMyCharts.bind(this);
- }
-
- userIdForFetch() {
- return this.state.showOnlyMyCharts ? this.props.userId : undefined;
- }
-
- componentDidMount() {
- this.slicesRequest = this.props.fetchSlices(
- this.userIdForFetch(),
- '',
- this.state.sortBy,
- );
- }
-
- componentDidUpdate(prevProps: SliceAdderProps) {
- const nextState: SliceAdderState = {} as SliceAdderState;
- if (this.props.lastUpdated !== prevProps.lastUpdated) {
- nextState.filteredSlices = getFilteredSortedSlices(
- this.props.slices,
- this.state.searchTerm,
- this.state.sortBy,
- this.state.showOnlyMyCharts,
- this.props.userId,
- );
- }
-
- if (prevProps.selectedSliceIds !== this.props.selectedSliceIds) {
- nextState.selectedSliceIdsSet = new Set(this.props.selectedSliceIds);
- }
-
- if (Object.keys(nextState).length) {
- this.setState(nextState);
- }
- }
- componentWillUnmount() {
- // Clears the redux store keeping only selected items
- const selectedSlices = pickBy(this.props.slices, (value: Slice) =>
- this.state.selectedSliceIdsSet.has(value.slice_id),
- );
-
- this.props.updateSlices(selectedSlices);
- if (this.slicesRequest instanceof AbortController) {
- this.slicesRequest.abort();
- }
- }
-
- handleChange = debounce(value => {
- this.searchUpdated(value);
- this.slicesRequest = this.props.fetchSlices(
- this.userIdForFetch(),
- value,
- this.state.sortBy,
- );
- }, 300);
-
- searchUpdated(searchTerm: string) {
- this.setState(prevState => ({
- searchTerm,
- filteredSlices: getFilteredSortedSlices(
- this.props.slices,
+function SliceAdder({
+ fetchSlices,
+ updateSlices,
+ isLoading,
+ slices,
+ errorMessage = '',
+ userId,
+ selectedSliceIds = [],
+ editMode = false,
+ dashboardId,
+}: SliceAdderProps) {
+ const theme = useTheme();
+ const slicesRequestRef = useRef<AbortController | Promise<void>>();
+
+ const [searchTerm, setSearchTerm] = useState('');
+ const [sortBy, setSortBy] = useState<keyof Slice>(DEFAULT_SORT_KEY);
+ const [selectedSliceIdsSet, setSelectedSliceIdsSet] = useState(
+ () => new Set(selectedSliceIds),
+ );
+
+ // Refs to track latest values for cleanup effect
+ const latestSlicesRef = useRef(slices);
+ const latestSelectedSliceIdsSetRef = useRef(selectedSliceIdsSet);
+ const [showOnlyMyCharts, setShowOnlyMyCharts] = useState(() =>
+ getItem(LocalStorageKeys.DashboardEditorShowOnlyMyCharts, true),
+ );
+
+ // Keep refs updated with latest values
+ useEffect(() => {
+ latestSlicesRef.current = slices;
+ }, [slices]);
+
+ useEffect(() => {
+ latestSelectedSliceIdsSetRef.current = selectedSliceIdsSet;
+ }, [selectedSliceIdsSet]);
+
+ const filteredSlices = useMemo(
+ () =>
+ getFilteredSortedSlices(
+ slices,
searchTerm,
- prevState.sortBy,
- prevState.showOnlyMyCharts,
- this.props.userId,
- ),
- }));
- }
-
- handleSelect(sortBy: keyof Slice) {
- this.setState(prevState => ({
- sortBy,
- filteredSlices: getFilteredSortedSlices(
- this.props.slices,
- prevState.searchTerm,
sortBy,
- prevState.showOnlyMyCharts,
- this.props.userId,
- ),
- }));
- this.slicesRequest = this.props.fetchSlices(
- this.userIdForFetch(),
- this.state.searchTerm,
- sortBy,
- );
- }
-
- rowRenderer({ index, style }: { index: number; style: React.CSSProperties })
{
- const { filteredSlices, selectedSliceIdsSet } = this.state;
- const cellData = filteredSlices[index];
-
- const isSelected = selectedSliceIdsSet.has(cellData.slice_id);
- const type = CHART_TYPE;
- const id = NEW_CHART_ID;
-
- const meta = {
- chartId: cellData.slice_id,
- sliceName: cellData.slice_name,
- };
- return (
- <DragDroppable
- key={cellData.slice_id}
- component={{ type, id, meta }}
- parentComponent={{
- id: NEW_COMPONENTS_SOURCE_ID,
- type: NEW_COMPONENT_SOURCE_TYPE,
- }}
- index={index}
- depth={0}
- disableDragDrop={isSelected}
- editMode={this.props.editMode}
- // we must use a custom drag preview within the List because
- // it does not seem to work within a fixed-position container
- useEmptyDragPreview
- // List library expect style props here
- // actual style should be applied to nested AddSliceCard component
- style={{}}
- >
- {({ dragSourceRef }: { dragSourceRef: ConnectDragSource }) => (
- <AddSliceCard
- innerRef={dragSourceRef}
- style={style}
- sliceName={cellData.slice_name}
- lastModified={cellData.changed_on_humanized}
- visType={cellData.viz_type}
- datasourceUrl={cellData.datasource_url}
- datasourceName={cellData.datasource_name}
- thumbnailUrl={cellData.thumbnail_url}
- isSelected={isSelected}
- />
- )}
- </DragDroppable>
- );
- }
-
- onShowOnlyMyCharts = (showOnlyMyCharts: boolean) => {
- if (!showOnlyMyCharts) {
- this.slicesRequest = this.props.fetchSlices(
- undefined,
- this.state.searchTerm,
- this.state.sortBy,
- );
- }
- this.setState(prevState => ({
- showOnlyMyCharts,
- filteredSlices: getFilteredSortedSlices(
- this.props.slices,
- prevState.searchTerm,
- prevState.sortBy,
showOnlyMyCharts,
- this.props.userId,
+ userId,
),
- }));
- setItem(LocalStorageKeys.DashboardEditorShowOnlyMyCharts,
showOnlyMyCharts);
- };
+ [slices, searchTerm, sortBy, showOnlyMyCharts, userId],
+ );
+
+ const userIdForFetch = useCallback(
+ () => (showOnlyMyCharts ? userId : undefined),
+ [showOnlyMyCharts, userId],
+ );
+
+ // componentDidMount
+ useEffect(() => {
+ slicesRequestRef.current = fetchSlices(userIdForFetch(), '', sortBy);
+ // Only run on mount
+ // eslint-disable-next-line react-hooks/exhaustive-deps
Review Comment:
<!-- Bito Reply -->
The ESLint rule `react-hooks/exhaustive-deps` is referenced in a disable
comment on line 225 of the file
`superset-frontend/src/dashboard/components/SliceAdder.tsx`. The suggestion
indicates that the rule definition is not found, which could be due to the
`eslint-plugin-react-hooks` package not being properly installed or configured
in the ESLint setup. The disable comment is correct and active in CI, so it
should be addressed by ensuring the necessary package is installed and the
ESLint configuration is updated accordingly.
**superset-frontend/src/dashboard/components/SliceAdder.tsx**
```
// eslint-disable-next-line react-hooks/exhaustive-deps
```
--
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]
