Rewrite _commons to Typescript

[wenche]

Resolve #728

[wenche]

I'm having a hard time with this one.

useKeyPress.ts Doesn't seem like this is actually in use, should probably be deleted?

useCombinedRefs.ts I was trying this approach but it gives a lot of errors in the component using this function along with eslint complaining on any

setReactInputValue 🙈 This use private React stuffs....

useOutsideClick I think this one is OK 😓

[mimarz]

Would this work for useCombinedRefs ?

import { useRef, useEffect } from 'react'
type Ref = React.MutableRefObject<HTMLElement>
type useCombinedRefsProps<T> = T | ((a: HTMLElement) => T)
export const useCombinedRefs = (...refs: useCombinedRefsProps<Ref>[]): Ref => {
  const targetRef = useRef<HTMLElement>(null)
  useEffect(() => {
    refs.forEach((ref) => {
      if (!ref) return
      if (typeof ref === 'function') {
        ref(targetRef.current)
      } else {
        ref.current = targetRef.current
      }
    })
  }, [refs])
  return targetRef
}

[wenche]

Let's try!

[wenche]

Tried and there's no issues with the useCombinedRefs file itself. But the same problems in TabList and Menu remain.

[!] (plugin rpt2) Error: /Users/wenche/Documents/prosjekter/equinor/migrate-common/libraries/core-react/src/Tabs/TabList.tsx(73,27): semantic error TS2345: Argument of type 'LegacyRef<HTMLButtonElement>' is not assignable to parameter of type 'Ref<HTMLElement>'.
  Type 'string' is not assignable to type 'Ref<HTMLElement>'.

I don't know what the LegacyRef is 🤔 Have to google that

[!] (plugin rpt2) Error: /Users/wenche/Documents/prosjekter/equinor/migrate-common/libraries/core-react/src/Menu/MenuItem.tsx(124,9): semantic error TS2769: No overload matches this call.
  Overload 1 of 2, '(props: Pick<Pick<Pick<DetailedHTMLProps<LiHTMLAttributes<HTMLLIElement>, HTMLLIElement>, "slot" | "style" | ... 253 more ... | "value"> & { ...; } & StyleProps, "active" | ... 257 more ... | "value"> & Partial<...>, "active" | ... 257 more ... | "value"> & { ...; } & { ...; }): ReactElement<...>', gave the following error.
    Type 'Ref' is not assignable to type 'Ref<HTMLLIElement>'.
      Type 'MutableRefObject<HTMLElement>' is not assignable to type 'RefObject<HTMLLIElement>'.
        Types of property 'current' are incompatible.
          Type 'HTMLElement' is missing the following properties from type 'HTMLLIElement': type, value
  Overload 2 of 2, '(props: StyledComponentPropsWithAs<"li", any, StyleProps, never>): ReactElement<StyledComponentPropsWithAs<"li", any, StyleProps, never>, string | ... 1 more ... | (new (props: any) => Component<...>)>', gave the following error.
    Type 'Ref' is not assignable to type 'Ref<HTMLLIElement>'.
      Type 'MutableRefObject<HTMLElement>' is not assignable to type 'RefObject<HTMLLIElement>'.

[mimarz]

It is what it is, a hack. React does not provide internal types (that I could find) so we have to make it work with just faking it.

Suggestion:

// Workaround
// React ignores 'dispathEvent' on input/textarea, see https://github.com/facebook/react/issues/10135
type ReactInternalHack = { _valueTracker?: { setValue: (a: string) => void } }

export const setReactInputValue = (
  input: HTMLInputElement & ReactInternalHack,
  value: string,
): void => {
  const previousValue = input.value

  input.value = value

  const tracker = input._valueTracker

  if (typeof tracker !== 'undefined') {
    tracker.setValue(previousValue)
  }

  //'change' instead of 'input', see https://github.com/facebook/react/issues/11488#issuecomment-381590324
  input.dispatchEvent(new Event('change', { bubbles: true }))
}

You will also need to cast the input on line 217, in <Search /> to HTMLInputElement for now...

[mimarz]

I had a second look at useCombinedRefs as we got a the wrong type in return when using it in for example Search.

New suggested useCombinedRefs

import { useRef, useEffect, MutableRefObject } from 'react'

type Ref<T> = MutableRefObject<T>
type Props<T, P> = T | ((a: P) => void)

export const useCombinedRefs = <T extends HTMLElement>(
  ...refs: Props<Ref<T>, T>[]
): Ref<T> => {
  const targetRef = useRef<T>(null)
  useEffect(() => {
    refs.forEach((ref) => {
      if (!ref) return
      if (typeof ref === 'function') {
        ref(targetRef.current)
      } else {
        ref.current = targetRef.current
      }
    })
  }, [refs])
  return targetRef
}

This means that when using useCombinedRefs you will have to set which type the ref is.
Example in <Search/>

const inputRef = useCombinedRefs<HTMLInputElement>(
      useRef<HTMLInputElement>(null),
      ref,
    )

[wenche]

I'm not sure about this one

type TabChild = ButtonHTMLAttributes<HTMLButtonElement> &
  RefAttributes<HTMLButtonElement> &
  ReactElement

but it did remove the ts errors 🤷‍♀️

[mimarz]

I'm not sure about this one

type TabChild = ButtonHTMLAttributes<HTMLButtonElement> &
  RefAttributes<HTMLButtonElement> &
  ReactElement

but it did remove the ts errors 🤷‍♀️

The tabs are ButtonElement so why not 🙈 Lets test 🙂

[wenche]

@mimarz What do you suggest for this error?

[mimarz]

This looks to be the wrong type (from ts convert). el argument should be HTMLLIElement? Might aswell fix it now since you are here 😅

[mimarz]

Just one small type fix from before we could fix while touching this file, else it looks good 👍

[mimarz]

LGTM 👍