From 1de36ab4fcd47d8b91abcf2a79d367a1cf60f1a9 Mon Sep 17 00:00:00 2001 From: Ben Grynhaus Date: Wed, 15 Apr 2020 01:51:40 +0300 Subject: [PATCH 1/5] better TypeScript modifier types for `usePopper` --- typings/tests/main-test.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/typings/tests/main-test.tsx b/typings/tests/main-test.tsx index 8af4ba6..e993623 100644 --- a/typings/tests/main-test.tsx +++ b/typings/tests/main-test.tsx @@ -60,7 +60,12 @@ const HookTest = () => { referenceElement, popperElement, { - modifiers: [{ name: 'arrow', options: { element: arrowElement } }], + modifiers: [ + { + name: 'arrow', + options: { element: arrowElement as HTMLElement | undefined }, + }, + ], } ); From a8e1da4875d8a3b2f2c9c948516ef48d9593abea Mon Sep 17 00:00:00 2001 From: Federico Zivolo Date: Wed, 15 Apr 2020 11:24:26 +0200 Subject: [PATCH 2/5] fix: popper-core 2.3.3 type fixes --- src/__typings__/main-test.js | 60 ++++++++++++++++++++++++++++++++++-- src/usePopper.js | 31 +++++++++++++------ yarn.lock | 2 +- 3 files changed, 80 insertions(+), 13 deletions(-) diff --git a/src/__typings__/main-test.js b/src/__typings__/main-test.js index bdabe39..865f52d 100644 --- a/src/__typings__/main-test.js +++ b/src/__typings__/main-test.js @@ -4,7 +4,7 @@ // be found under `/typings/tests` please. Thanks! 🤗 import React from 'react'; -import { Manager, Reference, Popper } from '..'; +import { Manager, Reference, Popper, usePopper } from '..'; export const Test = () => ( @@ -33,7 +33,10 @@ export const Test = () => ( }) => (
update()} > @@ -51,3 +54,56 @@ export const Test = () => ( ); + +export const HookTest = () => { + const [referenceElement, setReferenceElement] = React.useState( + null + ); + const [popperElement, setPopperElement] = React.useState(null); + const [arrowElement, setArrowElement] = React.useState(null); + const { styles, attributes, update } = usePopper( + referenceElement, + popperElement, + { + modifiers: [ + { + name: 'arrow', + options: { element: arrowElement || undefined }, + }, + ], + } + ); + + usePopper( + referenceElement, + popperElement, + // $FlowExpectError + { + modifiers: [ + { + name: 'offset', + options: { offset: [0, ''] }, + }, + ], + } + ); + + return ( + <> + + +
+ Popper element +
+
+ + ); +}; diff --git a/src/usePopper.js b/src/usePopper.js index 762170e..76dc1f0 100644 --- a/src/usePopper.js +++ b/src/usePopper.js @@ -2,14 +2,16 @@ import * as React from 'react'; import { createPopper as defaultCreatePopper, - type Options as PopperOptions, type VirtualElement, + type Modifier, + type OptionsGeneric, + type StrictModifiers, } from '@popperjs/core'; import isEqual from 'react-fast-compare'; import { fromEntries, useIsomorphicLayoutEffect } from './utils'; -type Options = $Shape<{ - ...PopperOptions, +type Options = $Shape<{ + ...OptionsGeneric, createPopper: typeof defaultCreatePopper, }>; @@ -22,14 +24,20 @@ type State = { }, }; +type UpdateStateModifier = Modifier<'updateState', {||}>; + const EMPTY_MODIFIERS = []; -export const usePopper = ( +export const usePopper = < + TModifiers: StrictModifiers | $Shape> +>( referenceElement: ?(Element | VirtualElement), popperElement: ?HTMLElement, - options: Options = {} + options: Options = {} ) => { - const prevOptions = React.useRef(null); + type TExtendedModifier = TModifiers | $Shape; + + const prevOptions = React.useRef>(null); const optionsWithDefaults = { onFirstUpdate: options.onFirstUpdate, @@ -49,7 +57,7 @@ export const usePopper = ( attributes: {}, }); - const updateStateModifier = React.useMemo( + const updateStateModifier = React.useMemo( () => ({ name: 'updateState', enabled: true, @@ -71,7 +79,7 @@ export const usePopper = ( [] ); - const popperOptions = React.useMemo(() => { + const popperOptions = React.useMemo>(() => { const newOptions = { onFirstUpdate: optionsWithDefaults.onFirstUpdate, placement: optionsWithDefaults.placement, @@ -83,8 +91,11 @@ export const usePopper = ( ], }; - if (isEqual(prevOptions.current, newOptions)) { - return prevOptions.current || newOptions; + if ( + prevOptions.current != null && + isEqual(prevOptions.current, newOptions) + ) { + return prevOptions.current; } else { prevOptions.current = newOptions; return newOptions; diff --git a/yarn.lock b/yarn.lock index 5749a3a..172b815 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1253,7 +1253,7 @@ "@popperjs/core@^2.3.3": version "2.3.3" - resolved "https://registry.npmjs.org/@popperjs/core/-/core-2.3.3.tgz#8731722aeb7330e8fd9eb5d424be6b98dea7d6da" + resolved "https://registry.yarnpkg.com/@popperjs/core/-/core-2.3.3.tgz#8731722aeb7330e8fd9eb5d424be6b98dea7d6da" integrity sha512-yEvVC8RfhRPkD9TUn7cFcLcgoJePgZRAOR7T21rcRY5I8tpuhzeWfGa7We7tB14fe9R7wENdqUABcMdwD4SQLw== "@rollup/plugin-commonjs@^11.0.2": From 9bae7dbd19ac1a36b6fd61c1cfeed411bc9479dc Mon Sep 17 00:00:00 2001 From: Federico Zivolo Date: Thu, 16 Apr 2020 13:40:59 +0200 Subject: [PATCH 3/5] fix: more Flow fixes --- .flowconfig | 2 +- src/__typings__/main-test.js | 33 ++++++++++++++++++++++++++++----- src/usePopper.js | 15 ++++++++------- 3 files changed, 37 insertions(+), 13 deletions(-) diff --git a/.flowconfig b/.flowconfig index 0d47238..fdd7016 100644 --- a/.flowconfig +++ b/.flowconfig @@ -9,7 +9,7 @@ [lints] [options] -suppress_comment=\\(.\\|\n\\)*\\$FlowExpectError +suppress_comment=\\(.\\|\n\\)*\\$\\(FlowExpectError\\|FlowFixMe\\) [strict] diff --git a/src/__typings__/main-test.js b/src/__typings__/main-test.js index 865f52d..059fcc7 100644 --- a/src/__typings__/main-test.js +++ b/src/__typings__/main-test.js @@ -5,6 +5,7 @@ import React from 'react'; import { Manager, Reference, Popper, usePopper } from '..'; +import type { Modifier, StrictModifiers } from '@popperjs/core'; export const Test = () => ( @@ -68,21 +69,43 @@ export const HookTest = () => { modifiers: [ { name: 'arrow', - options: { element: arrowElement || undefined }, + options: { element: arrowElement }, }, ], } ); - usePopper( + usePopper(referenceElement, popperElement, { + modifiers: [ + // $FlowExpectError: offset tuple accepts only numbers + { + name: 'offset', + options: { offset: [0, ''] }, + }, + ], + }); + + Number(2 + null); + + type CustomModifier = $Shape>; + usePopper(referenceElement, popperElement, { + modifiers: [ + { + name: 'custom', + options: { foo: true }, + }, + ], + }); + + usePopper( referenceElement, popperElement, - // $FlowExpectError + // $FlowExpectError: foo should be boolean { modifiers: [ { - name: 'offset', - options: { offset: [0, ''] }, + name: 'custom', + options: { foo: 'str' }, }, ], } diff --git a/src/usePopper.js b/src/usePopper.js index 76dc1f0..f5743df 100644 --- a/src/usePopper.js +++ b/src/usePopper.js @@ -28,16 +28,16 @@ type UpdateStateModifier = Modifier<'updateState', {||}>; const EMPTY_MODIFIERS = []; -export const usePopper = < - TModifiers: StrictModifiers | $Shape> ->( +type DefaultModifiers = StrictModifiers | $Shape>; + +export const usePopper = ( referenceElement: ?(Element | VirtualElement), popperElement: ?HTMLElement, - options: Options = {} + options: Options = {} ) => { - type TExtendedModifier = TModifiers | $Shape; + type InternalModifiers = Modifiers | $Shape; - const prevOptions = React.useRef>(null); + const prevOptions = React.useRef>(null); const optionsWithDefaults = { onFirstUpdate: options.onFirstUpdate, @@ -79,7 +79,7 @@ export const usePopper = < [] ); - const popperOptions = React.useMemo>(() => { + const popperOptions = React.useMemo>(() => { const newOptions = { onFirstUpdate: optionsWithDefaults.onFirstUpdate, placement: optionsWithDefaults.placement, @@ -97,6 +97,7 @@ export const usePopper = < ) { return prevOptions.current; } else { + // $FlowFixMe: Cannot assign `newOptions` to `prevOptions.current` because string [1] is incompatible with string literal `updateState` [2] in property `name` of array element of property `modifiers` prevOptions.current = newOptions; return newOptions; } From be38e84aa7cf9a65fca598d7f4a8e14f06db414e Mon Sep 17 00:00:00 2001 From: Federico Zivolo Date: Tue, 21 Apr 2020 16:50:30 +0200 Subject: [PATCH 4/5] expose all Flow errors --- src/__typings__/main-test.js | 2 -- src/usePopper.js | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/__typings__/main-test.js b/src/__typings__/main-test.js index 059fcc7..e6e46fc 100644 --- a/src/__typings__/main-test.js +++ b/src/__typings__/main-test.js @@ -85,8 +85,6 @@ export const HookTest = () => { ], }); - Number(2 + null); - type CustomModifier = $Shape>; usePopper(referenceElement, popperElement, { modifiers: [ diff --git a/src/usePopper.js b/src/usePopper.js index f5743df..fbd0207 100644 --- a/src/usePopper.js +++ b/src/usePopper.js @@ -97,7 +97,6 @@ export const usePopper = ( ) { return prevOptions.current; } else { - // $FlowFixMe: Cannot assign `newOptions` to `prevOptions.current` because string [1] is incompatible with string literal `updateState` [2] in property `name` of array element of property `modifiers` prevOptions.current = newOptions; return newOptions; } @@ -123,7 +122,7 @@ export const usePopper = ( } const createPopper = options.createPopper || defaultCreatePopper; - const popperInstance = createPopper( + const popperInstance = createPopper( referenceElement, popperElement, popperOptions From f79adea2634d5009f1f0c77cf1727a8562f642ee Mon Sep 17 00:00:00 2001 From: Federico Zivolo Date: Tue, 21 Apr 2020 16:54:49 +0200 Subject: [PATCH 5/5] merge conflict --- typings/tests/main-test.tsx | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/typings/tests/main-test.tsx b/typings/tests/main-test.tsx index e993623..8af4ba6 100644 --- a/typings/tests/main-test.tsx +++ b/typings/tests/main-test.tsx @@ -60,12 +60,7 @@ const HookTest = () => { referenceElement, popperElement, { - modifiers: [ - { - name: 'arrow', - options: { element: arrowElement as HTMLElement | undefined }, - }, - ], + modifiers: [{ name: 'arrow', options: { element: arrowElement } }], } );