Skip to content

Commit

Permalink
[Linting] Fixed useExhaustiveDependencies in biome (#3197)
Browse files Browse the repository at this point in the history
* feat: Update biome

* feat: migrate biome config

* misc: Ignore semantic-tag rule biome

* refactor: resolved breaking changes after biome update

* feat: remove old excluded biome rule

* feat: Removed a11y/useFocusableInteractive ignore

* refactor: Fix useExhaustiveDependencies  biome

* memo: changeset

* memo: changeset gitmoji

* bug: Re-add selectedOptions to dependency array

* Update @navikt/core/react/src/overlays/dismissablelayer/DismissableLayer.tsx

Co-authored-by: Halvor Haugan <[email protected]>

* bug: Fix initial focus for sidebar icon

* ⚡ remove reduntant memoization of isLimitReached

* bug: re-add sanity-plugin-iframe-pane

---------

Co-authored-by: Halvor Haugan <[email protected]>
  • Loading branch information
KenAJoh and HalvorHaugan authored Oct 8, 2024
1 parent 324358d commit aac12fe
Show file tree
Hide file tree
Showing 19 changed files with 111 additions and 195 deletions.
5 changes: 5 additions & 0 deletions .changeset/bright-meals-repair.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@navikt/ds-react": patch
---

Performance: :zap: Optimized memoization for rerendring in some components.
2 changes: 1 addition & 1 deletion @navikt/aksel-icons/figma-plugin/src/ui/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const App = () => {
icon.removeEventListener("dragend", dragEvent);
}
};
}, [categories]);
}, []);

return (
<main tabIndex={-1} className="wrapper">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ const InputProvider = ({ children, value: props }: Props) => {
setInternalValue("");
setSearchTerm("");
},
[externalOnChange, onClear, setInternalValue],
[externalOnChange, onClear],
);

const focusInput = useCallback(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,10 @@ const SelectedOptionsProvider = ({
(!!maxSelected?.limit && selectedOptions.length >= maxSelected.limit) ||
(!isMultiSelect && selectedOptions.length > 0);

// biome-ignore lint/correctness/useExhaustiveDependencies: We explicitly want to run this effect when selectedOptions changes to match the view with the selected options.
useEffect(() => {
setHideCaret(isLimitReached);
}, [selectedOptions, setHideCaret, isLimitReached]);
}, [isLimitReached, selectedOptions, setHideCaret]);

const toggleOption = useCallback(
(
Expand Down
4 changes: 2 additions & 2 deletions @navikt/core/react/src/form/combobox/customOptionsContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const CustomOptionsProvider = ({
);
focusInput();
},
[focusInput, setCustomOptions],
[focusInput],
);

const addCustomOption = useCallback(
Expand All @@ -47,7 +47,7 @@ const CustomOptionsProvider = ({
}
focusInput();
},
[focusInput, isMultiSelect, setCustomOptions],
[focusInput, isMultiSelect],
);

const customOptionsState = {
Expand Down
4 changes: 2 additions & 2 deletions @navikt/core/react/src/modal/Modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ export const Modal = forwardRef<HTMLDialogElement, ModalProps>(
// We have to use JS because it doesn't work to set it with a prop (React bug?)
// Currently doesn't seem to work in Chrome. See also Tooltip.tsx
if (modalRef.current && portalNode) modalRef.current.autofocus = true;
}, [modalRef, portalNode]);
}, [portalNode]);

useEffect(() => {
// We need to have this in a useEffect so that the content renders before the modal is displayed,
Expand All @@ -140,7 +140,7 @@ export const Modal = forwardRef<HTMLDialogElement, ModalProps>(
modalRef.current.close();
}
}
}, [modalRef, portalNode, open]);
}, [portalNode, open]);

useBodyScrollLock(modalRef, portalNode, isNested);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,8 @@ const DismissableLayerNode = forwardRef<HTMLDivElement, DismissableLayerProps>(
* If `disableOutsidePointerEvents` is true,
* we want to disable pointer events on the body when the first layer is opened.
*/

// biome-ignore lint/correctness/useExhaustiveDependencies: Every time the descendants change, we want to update the body pointer events since we might have added or removed a layer.
useEffect(() => {
if (!node || !enabled || !disableOutsidePointerEvents) return;

Expand All @@ -362,6 +364,7 @@ const DismissableLayerNode = forwardRef<HTMLDivElement, DismissableLayerProps>(
/**
* To make sure pointerEvents are enabled for all parents and siblings when the layer is removed from the DOM
*/
// biome-ignore lint/correctness/useExhaustiveDependencies: We explicitly want to run this on unmount, including every time the node updates to make sure we don't lock the application behind pointer-events: none.
useEffect(() => {
return () => descendants.values().forEach((x) => x.forceUpdate());
}, [descendants, node]);
Expand Down
31 changes: 15 additions & 16 deletions @navikt/core/react/src/table/AnimateHeight.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,37 +74,36 @@ const AnimateHeight: React.FC<AnimateHeightProps> = ({
const animationClassesTimeoutID = useRef<Timeout>();
const timeoutID = useRef<Timeout>();

const duration = prefersReducedMotion ? 0 : userDuration;
const initialHeight = useRef<Height>(height);
const initialOverflow = useRef<Overflow>("visible");

let initHeight: Height = height;
let initOverflow: Overflow = "visible";
const duration = prefersReducedMotion ? 0 : userDuration;

if (typeof initHeight === "number") {
if (typeof initialHeight.current === "number") {
// Reset negative height to 0
if (typeof height !== "string") {
initHeight = height < 0 ? 0 : height;
initialHeight.current = height < 0 ? 0 : height;
}
initOverflow = "hidden";
} else if (isPercentage(initHeight)) {
initialOverflow.current = "hidden";
} else if (isPercentage(initialHeight.current)) {
// If value is string "0%" make sure we convert it to number 0
initHeight = height === "0%" ? 0 : height;
initOverflow = "hidden";
initialHeight.current = height === "0%" ? 0 : height;
initialOverflow.current = "hidden";
}

const [currentHeight, setCurrentHeight] = useState<Height>(initHeight);
const [overflow, setOverflow] = useState<Overflow>(initOverflow);
const [currentHeight, setCurrentHeight] = useState<Height>(
initialHeight.current,
);
const [overflow, setOverflow] = useState<Overflow>(initialOverflow.current);
const [useTransitions, setUseTransitions] = useState<boolean>(false);

// ------------------ Did mount
useEffect(() => {
// Hide content if height is 0 (to prevent tabbing into it)
hideContent(contentElement.current, currentHeight);

// This should be explicitly run only on mount
// eslint-disable-next-line react-hooks/exhaustive-deps
hideContent(contentElement.current, initialHeight.current);
}, []);

// ------------------ Height update
// biome-ignore lint/correctness/useExhaustiveDependencies: This should be explicitly run only on height change
useEffect(() => {
if (height !== prevHeight.current && contentElement.current) {
showContent(contentElement.current, prevHeight.current);
Expand Down
1 change: 1 addition & 0 deletions @navikt/core/react/src/util/TextareaAutoSize.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ const TextareaAutosize = forwardRef<HTMLTextAreaElement, TextareaAutosizeProps>(
syncHeight();
});

// biome-ignore lint/correctness/useExhaustiveDependencies: Since value is an external prop, we want to reset the renders on every time it changes, even when it is undefined or empty.
useEffect(() => {
renders.current = 0;
}, [value]);
Expand Down
5 changes: 3 additions & 2 deletions @navikt/core/react/src/util/create-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ export function createContext<T>(options: CreateContextOptions<T> = {}) {
const Provider = forwardRef<unknown, ProviderProps<T>>(
({ children, ...context }, ref) => {
// Only re-memoize when prop values change
// eslint-disable-next-line react-hooks/exhaustive-deps
const value = React.useMemo(() => context, Object.values(context)) as T;

// biome-ignore lint/correctness/useExhaustiveDependencies: Object.values(context) includes all dependencies.
const value = React.useMemo(() => context, Object.values(context)) as T; // eslint-disable-line react-hooks/exhaustive-deps

return (
<Context.Provider value={ref ? { ...value, ref } : value}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ function Select({ children }: { children?: React.ReactNode }) {
const descendants = useDescendants();
const count = descendants.count();

// biome-ignore lint/correctness/useExhaustiveDependencies: We want count to be a trigger for focusing the last descendant here.
React.useEffect(() => {
descendants.last()?.node.focus();
}, [descendants, count]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export const IconSidebar = ({
const wrapperRef = useRef<HTMLDivElement>(null);
const [blob, setBlob]: any = useState();
const router = useRouter();
const prevNameRef = useRef<string | undefined>();

const currentIcon = useMemo(
() => Object.values(meta).find((x) => x.name === name),
Expand All @@ -44,12 +45,13 @@ export const IconSidebar = ({
}

useEffect(() => {
if (wrapperRef.current) {
if (prevNameRef.current !== name && wrapperRef.current) {
wrapperRef.current.focus({ preventScroll: true });
}
prevNameRef.current = name;
}, [name]);

const logDownload = (icon, format) => {
const logDownload = (icon: string, format: "svg") => {
amplitude.track(AmplitudeEvents.ikonnedlastning, {
icon,
format,
Expand Down
1 change: 1 addition & 0 deletions aksel.nav.no/website/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
"react-rx": "^2.1.3",
"remark-gfm": "^4.0.0",
"sanity": "3.59.0",
"sanity-plugin-iframe-pane": "^3.1.6",
"sanity-plugin-media": "^2.3.2",
"sharp": "0.32.6",
"styled-components": "^6.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ const Example = () => {
const [hasError, setHasError] = useState(false);

useEffect(() => {
errorRef.current && errorRef.current.focus();
if (hasError && errorRef.current) {
errorRef.current.focus();
}
}, [hasError]);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ const Example = () => {
}

useEffect(() => {
errorSummaryRef.current?.focus();
formState.tries > 0 && errorSummaryRef.current?.focus();
}, [formState.tries]);

if (formState.submitted)
Expand Down
129 changes: 0 additions & 129 deletions aksel.nav.no/website/sanity/plugins/structure/IFrame.tsx

This file was deleted.

Loading

0 comments on commit aac12fe

Please sign in to comment.