Merge pull request #2167 from Shopify/hookify-fvs

[FilterValueSelector] Convert to functional component & add isMountedRef

UNRELEASED.md

@@ -20,6 +20,9 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
 
 ### Code quality
 
+- Migrated `FilterValueSelector` to use hooks instead of withAppProvider ([#2156](https://github.com/Shopify/polaris-react/pull/2156))
+- Added `useIsMountedRef` hook to use while building components ([#2167](https://github.com/Shopify/polaris-react/pull/2167))
+
 ### Deprecations
 
 ### Development workflow

src/components/ResourceList/components/FilterControl/components/FilterValueSelector/FilterValueSelector.tsx

@@ -1,13 +1,11 @@
-import React from 'react';
+import React, {useCallback} from 'react';
 import {Select} from '../../../../../Select';
 import {Stack} from '../../../../../Stack';
 import {TextField} from '../../../../../TextField';
-import {
-  withAppProvider,
-  WithAppProviderProps,
-} from '../../../../../../utilities/with-app-provider';
 import {DateSelector} from '../DateSelector';
 import {Filter, AppliedFilter, FilterType, Operator} from '../../types';
+import {useI18n} from '../../../../../../utilities/i18n';
+import {useIsMountedRef} from '../../../../../../utilities/use-is-mounted-ref';
 
 export interface FilterValueSelectorProps {
   filter: Filter;
@@ -17,110 +15,94 @@ export interface FilterValueSelectorProps {
   onFilterKeyChange(filterKey: string): void;
 }
 
-type CombinedProps = FilterValueSelectorProps & WithAppProviderProps;
-
-class FilterValueSelector extends React.PureComponent<CombinedProps> {
-  componentDidMount() {
-    const {
-      filter: {operatorText, type},
-    } = this.props;
-
-    if (
-      type === FilterType.DateSelector ||
-      !operatorText ||
-      typeof operatorText === 'string' ||
-      operatorText.length === 0
-    ) {
-      return;
-    }
-
-    this.handleOperatorOptionChange(operatorText[0].key);
+export function FilterValueSelector({
+  filter,
+  filterKey,
+  value,
+  onChange,
+  onFilterKeyChange,
+}: FilterValueSelectorProps) {
+  const {translate} = useI18n();
+  const isMounted = useIsMountedRef();
+  const {operatorText, type, label} = filter;
+  const showOperatorOptions =
+    type !== FilterType.DateSelector &&
+    operatorText &&
+    typeof operatorText !== 'string';
+
+  const handleOperatorOptionChange = useCallback(
+    (operatorKey: string) => {
+      onFilterKeyChange(operatorKey);
+
+      if (!value) {
+        return;
+      }
+
+      onChange(value);
+    },
+    [onChange, onFilterKeyChange, value],
+  );
+
+  if (showOperatorOptions && operatorText!.length !== 0 && !isMounted.current) {
+    handleOperatorOptionChange((operatorText as Operator[])[0].key);
   }
 
-  render() {
-    const {
-      filter,
-      filterKey,
-      value,
-      onChange,
-      onFilterKeyChange,
-      polaris: {intl},
-    } = this.props;
-
-    const {operatorText} = filter;
-
-    const showOperatorOptions =
-      filter.type !== FilterType.DateSelector &&
-      operatorText &&
-      typeof operatorText !== 'string';
-    const operatorOptionsMarkup = showOperatorOptions ? (
-      <Select
-        label={filter.label}
-        labelHidden
-        options={buildOperatorOptions(operatorText)}
-        value={filterKey}
-        onChange={this.handleOperatorOptionChange}
-      />
-    ) : null;
-
-    const selectedFilterLabel =
-      typeof operatorText === 'string' ? operatorText : '';
-
-    switch (filter.type) {
-      case FilterType.Select:
-        return (
-          <Stack vertical>
-            {operatorOptionsMarkup}
-            <Select
-              label={selectedFilterLabel}
-              options={filter.options}
-              placeholder={intl.translate(
-                'Polaris.ResourceList.FilterValueSelector.selectFilterValuePlaceholder',
-              )}
-              value={value}
-              onChange={onChange}
-            />
-          </Stack>
-        );
-      case FilterType.TextField:
-        return (
-          <Stack vertical>
-            {operatorOptionsMarkup}
-            <TextField
-              label={selectedFilterLabel}
-              value={value}
-              type={filter.textFieldType}
-              onChange={onChange}
-            />
-          </Stack>
-        );
-      case FilterType.DateSelector:
-        return (
-          <DateSelector
-            dateOptionType={filter.dateOptionType}
-            filterValue={value}
-            filterKey={filterKey}
-            filterMinKey={filter.minKey}
-            filterMaxKey={filter.maxKey}
-            onFilterValueChange={onChange}
-            onFilterKeyChange={onFilterKeyChange}
+  const operatorOptionsMarkup = showOperatorOptions ? (
+    <Select
+      label={label}
+      labelHidden
+      options={buildOperatorOptions(operatorText)}
+      value={filterKey}
+      onChange={handleOperatorOptionChange}
+    />
+  ) : null;
+
+  const selectedFilterLabel =
+    typeof operatorText === 'string' ? operatorText : '';
+
+  switch (filter.type) {
+    case FilterType.Select:
+      return (
+        <Stack vertical>
+          {operatorOptionsMarkup}
+          <Select
+            label={selectedFilterLabel}
+            options={filter.options}
+            placeholder={translate(
+              'Polaris.ResourceList.FilterValueSelector.selectFilterValuePlaceholder',
+            )}
+            value={value}
+            onChange={onChange}
+          />
+        </Stack>
+      );
+    case FilterType.TextField:
+      return (
+        <Stack vertical>
+          {operatorOptionsMarkup}
+          <TextField
+            label={selectedFilterLabel}
+            value={value}
+            type={filter.textFieldType}
+            onChange={onChange}
           />
-        );
-      default:
-        return null;
-    }
+        </Stack>
+      );
+    case FilterType.DateSelector:
+      return (
+        <DateSelector
+          dateOptionType={filter.dateOptionType}
+          filterValue={value}
+          filterKey={filterKey}
+          filterMinKey={filter.minKey}
+          filterMaxKey={filter.maxKey}
+          onFilterValueChange={onChange}
+          onFilterKeyChange={onFilterKeyChange}
+        />
+      );
+    default:
+      return null;
   }
-
-  private handleOperatorOptionChange = (operatorKey: string) => {
-    const {value, onChange, onFilterKeyChange} = this.props;
-    onFilterKeyChange(operatorKey);
-
-    if (!value) {
-      return;
-    }
-
-    onChange(value);
-  };
 }
 
 function buildOperatorOptions(operatorText?: string | Operator[]) {
@@ -132,7 +114,3 @@ function buildOperatorOptions(operatorText?: string | Operator[]) {
     return {value: key, label: optionLabel};
   });
 }
-
-// Use named export once withAppProvider is refactored away
-// eslint-disable-next-line import/no-default-export
-export default withAppProvider<FilterValueSelectorProps>()(FilterValueSelector);

src/components/ResourceList/components/FilterControl/components/FilterValueSelector/index.ts

@@ -1,5 +1,4 @@
-import FilterValueSelector, {
+export {
+  FilterValueSelector,
   FilterValueSelectorProps,
 } from './FilterValueSelector';
-
-export {FilterValueSelector, FilterValueSelectorProps};

src/components/ResourceList/components/FilterControl/components/FilterValueSelector/tests/FilterValueSelector.test.tsx

@@ -1,7 +1,7 @@
 import React from 'react';
 import {trigger, mountWithAppProvider} from 'test-utilities/legacy';
 import {Select, TextField} from 'components';
-import FilterValueSelector from '../FilterValueSelector';
+import {FilterValueSelector} from '../FilterValueSelector';
 import {DateSelector} from '../../DateSelector';
 import {Filter, FilterType, Operator} from '../../../types';
 

src/utilities/tests/use-is-mounted-ref.test.tsx

@@ -0,0 +1,58 @@
+import React, {useEffect} from 'react';
+import {mount} from 'test-utilities';
+import {useIsMountedRef} from '../use-is-mounted-ref';
+
+describe('useIsMountedRef', () => {
+  it('returns a false value before mounting', () => {
+    const spy = jest.fn((_) => {});
+
+    function MockComponent() {
+      const isMounted = useIsMountedRef();
+      spy(isMounted.current);
+      return null;
+    }
+
+    mount(<MockComponent />);
+    expect(spy).toHaveBeenCalledWith(false);
+  });
+
+  it('returns a false value after unmounting', () => {
+    const spy = jest.fn((_) => {});
+
+    function MockComponent() {
+      const isMounted = useIsMountedRef();
+
+      useEffect(() => {
+        return () => {
+          // This ref is not pointing to a node rendered by react so
+          // we're ok ignoring the linting rule since it doesn't apply
+          // eslint-disable-next-line react-hooks/exhaustive-deps
+          spy(isMounted.current);
+        };
+      }, [isMounted]);
+
+      return null;
+    }
+
+    const mockComponent = mount(<MockComponent />);
+    mockComponent.unmount();
+    expect(spy).toHaveBeenCalledWith(false);
+  });
+
+  it('returns true after the component mounts', () => {
+    const spy = jest.fn((_) => {});
+
+    function MockComponent() {
+      const isMounted = useIsMountedRef();
+
+      useEffect(() => {
+        spy(isMounted.current);
+      }, [isMounted]);
+
+      return null;
+    }
+
+    mount(<MockComponent />);
+    expect(spy).toHaveBeenCalledWith(true);
+  });
+});

src/utilities/use-is-mounted-ref.ts

@@ -0,0 +1,19 @@
+import {useRef, useEffect, MutableRefObject} from 'react';
+
+/**
+ * Returns a MutatableRefObject containing a boolean value that
+ * represents a components mounted status.
+ * @returns MutableRefObject<boolean> The mounted status
+ */
+export function useIsMountedRef(): MutableRefObject<boolean> {
+  const isMounted = useRef(false);
+
+  useEffect(() => {
+    isMounted.current = true;
+    return () => {
+      isMounted.current = false;
+    };
+  }, []);
+
+  return isMounted;
+}