From aa4cfd7b7c34c2d724405a3ecffde7fe6cf3b50f Mon Sep 17 00:00:00 2001 From: Constance Date: Mon, 7 Feb 2022 08:10:42 -0800 Subject: [PATCH] Upgrade Typescript to 4.5.3 to match Kibana (#5591) * Upgrade typescript to 4.5.3 - to match Kibana * Fix typescript err TS2339 Property 'MSInputMethodContext' does not exist on type 'Window & typeof globalThis' * Fix typescript error TS2783 Error: 'onClick' is specified more than once, so this usage will be overwritten. * Fix typescript errors TS2525 and TS7031 Errors: - Initializer provides no value for this binding element and the binding element has no default value. - Binding element '_onClick' implicitly has an 'any' type. Fixes: - Fix primary type issue, which was using `...` instead of `|| {}` - Remove unnecessary `_onClick` cast - Optional switch to optional chaining instead of && check * Fix typescript error TS2774 Error: - This condition will always return true since this function is always defined. Did you mean to call it instead? Fix: - [EuiPinnableListGroup] Remove unnecessary check, pinnable serves that purpose - [EuiOverlayMask] Remove unnecessary check, there's already an early return at the beginning of the block - [extra] DRY out portalTarget var - it's being used interchangably with overlayMaskNode.current * Fix catch error types (TS2571 & TS2345) Errors: - Object is of type 'unknown'. - Type 'unknown' is not assignable to type 'EuiMarkdownParseError | null' see https://stackoverflow.com/questions/68240884/error-object-inside-catch-is-of-type-unkown We could optionally disable `useUnknownInCatchVariables`, but I think it also does make sense to check for a valid Error instance if we're going to dive into it. * Update typescript related dependencies * Changelog * Update reactElementTypeExpanded replacement - it appears to have changed as of the recent upgrade, this keeps all props that output `ReactElement` the same as before * Expand string enums that are used within unions - Was previously working, with, e.g. `number | SomeStringEnum`, now needs a config to restore functionality * Revert shouldExtractValuesFromUnion - it's causing too many other unintended side effects (like turning `|`s into `,`s. There's probably no working around `number | SomeStringEnum` without stating the enum, it'll have to be a props doc changing going forward * Table auto props docs - spell out direction explicitly - enum no longer appears to be extracted by react-docgen * Fix other reactNodeTypeExpanded replace - appears to have also changed/shortened since the upgrade * Convert reactNodeTypeExpanded to a regex It appears as ``` string | number | boolean | {} | ReactElement | ReactNodeArray | ReactPortal ``` in some places, and ``` string | number | boolean | {} | ReactElement | ReactNodeArray | ReactPortal | ({} & string) | (ReactElement<...> & string) | (ReactNodeArray & string) | (ReactPortal & string) ``` in others - this regex should capture both scenarios * Portal auto props docs - spell out position explicitly - string enums within objects no longer appear to be expanded * Fix broken EuiButton props table - EuiButtonDisplay was bogarting the docgenInfo for the file, apparently - moving its declaration below EuiButton and giving EuiButton its a displayName seems to be required to get its docgenInfo working * Improve react-docgen-typescript wiki docs - Add warning about displayName bug found in https://github.com/elastic/eui/pull/5591/commits/dede73b05374f1bfa5d2d55c468494f03b2beac1#r799165839 - useful links to our custom react-docgen config/filters + react's docs - lint misc typos/grammar --- CHANGELOG.md | 4 + package.json | 12 +- scripts/babel/react-docgen-typescript.js | 5 +- .../guide_section/guide_section.tsx | 2 +- src/components/basic_table/basic_table.tsx | 2 +- src/components/basic_table/table_types.ts | 4 +- src/components/button/button.tsx | 160 +++++----- .../header/header_links/header_links.tsx | 7 +- .../pinnable_list_group.tsx | 2 +- .../markdown_editor/markdown_editor.tsx | 2 +- src/components/overlay_mask/overlay_mask.tsx | 11 +- src/components/portal/portal.tsx | 6 +- .../search_bar/query/default_syntax.ts | 3 +- src/components/search_bar/search_bar.tsx | 5 +- src/components/suggest/suggest_item.tsx | 2 +- wiki/component-development.md | 14 +- yarn.lock | 299 +++++++++++++----- 17 files changed, 353 insertions(+), 187 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ce2e31808f7..9827cdfa72a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ - Fixed `EuiInMemoryTable`'s `onTableChange` callback not returning the correct `sort.field` value on pagination ([#5588](https://github.com/elastic/eui/pull/5588)) - Fixed `EuiFilePicker` allowing files to be removed when `disabled` ([#5603](https://github.com/elastic/eui/pull/5603)) +**Breaking changes** + +- Upgraded TypeScript version to ~4.5.3 ([#5591](https://github.com/elastic/eui/pull/5591)) + ## [`46.2.0`](https://github.com/elastic/eui/tree/v46.2.0) - Updated `EuiDataGrid`s with scrolling content to always have a border around the grid body and any scrollbars ([#5563](https://github.com/elastic/eui/pull/5563)) diff --git a/package.json b/package.json index 031c344a787..bf429f9bcb8 100644 --- a/package.json +++ b/package.json @@ -106,7 +106,7 @@ "@babel/plugin-transform-runtime": "^7.11.0", "@babel/preset-env": "^7.11.0", "@babel/preset-react": "^7.10.4", - "@babel/preset-typescript": "^7.12.1", + "@babel/preset-typescript": "^7.16.7", "@cypress/code-coverage": "^3.9.12", "@cypress/react": "^5.10.3", "@cypress/webpack-dev-server": "^1.7.0", @@ -132,8 +132,8 @@ "@types/tabbable": "^3.1.0", "@types/url-parse": "^1.4.3", "@types/uuid": "^8.3.0", - "@typescript-eslint/eslint-plugin": "^5.10.0", - "@typescript-eslint/parser": "^5.10.0", + "@typescript-eslint/eslint-plugin": "^5.10.2", + "@typescript-eslint/parser": "^5.10.2", "argparse": "^2.0.1", "autoprefixer": "^9.8.6", "axe-core": "^4.1.1", @@ -205,7 +205,7 @@ "puppeteer": "^5.5.0", "raw-loader": "^4.0.1", "react": "^16.12.0", - "react-docgen-typescript": "^1.20.5", + "react-docgen-typescript": "^2.2.2", "react-dom": "^16.12.0", "react-helmet": "^6.1.0", "react-redux": "^7.2.1", @@ -228,7 +228,7 @@ "start-server-and-test": "^1.11.3", "style-loader": "^1.2.1", "terser-webpack-plugin": "^4.1.0", - "typescript": "4.1.3", + "typescript": "4.5.3", "uglifyjs-webpack-plugin": "^2.2.0", "url-loader": "^4.1.0", "webpack": "^4.46.0", @@ -247,6 +247,6 @@ "prop-types": "^15.5.0", "react": "^16.12", "react-dom": "^16.12", - "typescript": "~4.1.3" + "typescript": "~4.5.3" } } diff --git a/scripts/babel/react-docgen-typescript.js b/scripts/babel/react-docgen-typescript.js index 28998f8f684..d2246c99252 100644 --- a/scripts/babel/react-docgen-typescript.js +++ b/scripts/babel/react-docgen-typescript.js @@ -416,7 +416,6 @@ const intrinsicValuesRaw = [ * Replace ReactElement and ReactNode expanded types with ReactElement and ReactNode */ const reactElementTypeExpanded = - 'ReactElement ReactElement Component)>) | (new (props: any) => Component)>'; + 'ReactElement>'; -const reactNodeTypeExpanded = - 'string | number | boolean | {} | ReactElement ReactElement Component)>) | (new (props: any) => Component<...>)> | ... 5 more ... | (ReactPortal & string)'; +const reactNodeTypeExpanded = /(string \| number \| boolean \| {} \| ReactElement \| ReactNodeArray \| ReactPortal)( \| \({} & string\).+\(ReactPortal & string\))?/g; diff --git a/src-docs/src/components/guide_section/guide_section.tsx b/src-docs/src/components/guide_section/guide_section.tsx index ba056e7b8e0..755eca9a737 100644 --- a/src-docs/src/components/guide_section/guide_section.tsx +++ b/src-docs/src/components/guide_section/guide_section.tsx @@ -134,7 +134,7 @@ export const GuideSection: FunctionComponent = ({ const isPlaygroundUnsupported = typeof window !== 'undefined' && typeof document !== 'undefined' && - !!window.MSInputMethodContext && + !!(window as any).MSInputMethodContext && // @ts-ignore doesn't exist? !!document.documentMode; diff --git a/src/components/basic_table/basic_table.tsx b/src/components/basic_table/basic_table.tsx index 5e322df829a..9a69a992f08 100644 --- a/src/components/basic_table/basic_table.tsx +++ b/src/components/basic_table/basic_table.tsx @@ -172,7 +172,7 @@ export interface Criteria { */ sort?: { field: keyof T; - direction: Direction; + direction: 'asc' | 'desc'; }; } diff --git a/src/components/basic_table/table_types.ts b/src/components/basic_table/table_types.ts index 11a3eb8c7f0..29b7e85b823 100644 --- a/src/components/basic_table/table_types.ts +++ b/src/components/basic_table/table_types.ts @@ -7,7 +7,7 @@ */ import { ReactElement, ReactNode, TdHTMLAttributes } from 'react'; -import { Direction, HorizontalAlignment } from '../../services'; +import { HorizontalAlignment } from '../../services'; import { Pagination } from './pagination_bar'; import { Action } from './action_types'; import { Primitive } from '../../services/sort/comparators'; @@ -147,7 +147,7 @@ export interface EuiTableSortingType { */ sort?: { field: keyof T; - direction: Direction; + direction: 'asc' | 'desc'; }; /** * Enables/disables unsorting of table columns. Supported by EuiInMemoryTable. diff --git a/src/components/button/button.tsx b/src/components/button/button.tsx index 44532a9cc8f..29b45c16315 100644 --- a/src/components/button/button.tsx +++ b/src/components/button/button.tsx @@ -110,6 +110,82 @@ export interface EuiButtonProps extends EuiButtonContentProps, CommonProps { style?: CSSProperties; } +export type EuiButtonPropsForAnchor = PropsForAnchor< + EuiButtonProps, + { + buttonRef?: Ref; + } +>; + +export type EuiButtonPropsForButton = PropsForButton< + EuiButtonProps, + { + buttonRef?: Ref; + } +>; + +export type Props = ExclusiveUnion< + EuiButtonPropsForAnchor, + EuiButtonPropsForButton +>; + +/** + * EuiButton is largely responsible for providing relevant props + * and the logic for element-specific attributes + */ +export const EuiButton: FunctionComponent = ({ + isDisabled: _isDisabled, + disabled: _disabled, + href, + target, + rel, + type = 'button', + buttonRef, + ...rest +}) => { + const isHrefValid = !href || validateHref(href); + const disabled = _disabled || !isHrefValid; + const isDisabled = _isDisabled || !isHrefValid; + + const buttonIsDisabled = rest.isLoading || isDisabled || disabled; + const element = href && !isDisabled ? 'a' : 'button'; + + let elementProps = {}; + // Props for all elements + elementProps = { ...elementProps, isDisabled: buttonIsDisabled }; + // Element-specific attributes + if (element === 'button') { + elementProps = { ...elementProps, disabled: buttonIsDisabled }; + } + + const relObj: { + rel?: string; + href?: string; + type?: ButtonHTMLAttributes['type']; + target?: string; + } = {}; + + if (href && !buttonIsDisabled) { + relObj.href = href; + relObj.rel = getSecureRelForTarget({ href, target, rel }); + relObj.target = target; + } else { + relObj.type = type as ButtonHTMLAttributes['type']; + } + + return ( + + ); +}; +EuiButton.displayName = 'EuiButton'; + export type EuiButtonDisplayProps = EuiButtonProps & HTMLAttributes & { /** @@ -123,12 +199,13 @@ export type EuiButtonDisplayProps = EuiButtonProps & }; /** - * *INTERNAL ONLY* - * Component for displaying any element as a button - * EuiButton is largely responsible for providing relevant props - * and the logic for element-specific attributes + * EuiButtonDisplay is an internal-only component used for displaying + * any element as a button. + * NOTE: This component *must* be below EuiButton in the file and + * EuiButton must also set a displayName for react-docgen-typescript + * to correctly set EuiButton's docgenInfo and display a props table. */ -const EuiButtonDisplay = forwardRef( +export const EuiButtonDisplay = forwardRef( ( { element = 'button', @@ -218,77 +295,4 @@ const EuiButtonDisplay = forwardRef( ); } ); - EuiButtonDisplay.displayName = 'EuiButtonDisplay'; -export { EuiButtonDisplay }; - -export type EuiButtonPropsForAnchor = PropsForAnchor< - EuiButtonProps, - { - buttonRef?: Ref; - } ->; - -export type EuiButtonPropsForButton = PropsForButton< - EuiButtonProps, - { - buttonRef?: Ref; - } ->; - -export type Props = ExclusiveUnion< - EuiButtonPropsForAnchor, - EuiButtonPropsForButton ->; - -export const EuiButton: FunctionComponent = ({ - isDisabled: _isDisabled, - disabled: _disabled, - href, - target, - rel, - type = 'button', - buttonRef, - ...rest -}) => { - const isHrefValid = !href || validateHref(href); - const disabled = _disabled || !isHrefValid; - const isDisabled = _isDisabled || !isHrefValid; - - const buttonIsDisabled = rest.isLoading || isDisabled || disabled; - const element = href && !isDisabled ? 'a' : 'button'; - - let elementProps = {}; - // Props for all elements - elementProps = { ...elementProps, isDisabled: buttonIsDisabled }; - // Element-specific attributes - if (element === 'button') { - elementProps = { ...elementProps, disabled: buttonIsDisabled }; - } - - const relObj: { - rel?: string; - href?: string; - type?: ButtonHTMLAttributes['type']; - target?: string; - } = {}; - - if (href && !buttonIsDisabled) { - relObj.href = href; - relObj.rel = getSecureRelForTarget({ href, target, rel }); - relObj.target = target; - } else { - relObj.type = type as ButtonHTMLAttributes['type']; - } - - return ( - - ); -}; diff --git a/src/components/header/header_links/header_links.tsx b/src/components/header/header_links/header_links.tsx index b99996f1af7..d4325950a83 100644 --- a/src/components/header/header_links/header_links.tsx +++ b/src/components/header/header_links/header_links.tsx @@ -73,16 +73,15 @@ export const EuiHeaderLinks: FunctionComponent = ({ popoverProps, ...rest }) => { - const { onClick: _onClick, iconType = 'apps', ...popoverButtonRest } = { - ...popoverButtonProps, - }; + const { onClick, iconType = 'apps', ...popoverButtonRest } = + popoverButtonProps || {}; const [mobileMenuIsOpen, setMobileMenuIsOpen] = useState(false); const onMenuButtonClick: MouseEventHandler< HTMLButtonElement & HTMLAnchorElement > = (e) => { - _onClick && _onClick(e); + onClick?.(e); setMobileMenuIsOpen(!mobileMenuIsOpen); }; diff --git a/src/components/list_group/pinnable_list_group/pinnable_list_group.tsx b/src/components/list_group/pinnable_list_group/pinnable_list_group.tsx index 8e52194fd42..6b231d1038a 100644 --- a/src/components/list_group/pinnable_list_group/pinnable_list_group.tsx +++ b/src/components/list_group/pinnable_list_group/pinnable_list_group.tsx @@ -93,7 +93,7 @@ export const EuiPinnableListGroup: FunctionComponent ); // Add the pinning action unless the item has it's own extra action - if (onPinClick && !itemProps.extraAction && pinnable) { + if (pinnable && !itemProps.extraAction) { // Different displays for pinned vs unpinned if (pinned) { itemProps.extraAction = { diff --git a/src/components/markdown_editor/markdown_editor.tsx b/src/components/markdown_editor/markdown_editor.tsx index 10dfa296f1c..fd94fa5e3f4 100644 --- a/src/components/markdown_editor/markdown_editor.tsx +++ b/src/components/markdown_editor/markdown_editor.tsx @@ -254,7 +254,7 @@ export const EuiMarkdownEditor = forwardRef< const parsed = parser.processSync(value); return [parsed, null]; } catch (e) { - return [null, e]; + return [null, e as EuiMarkdownParseError]; } }, [parser, value]); diff --git a/src/components/overlay_mask/overlay_mask.tsx b/src/components/overlay_mask/overlay_mask.tsx index 6158e5fec27..114fa01dc8a 100644 --- a/src/components/overlay_mask/overlay_mask.tsx +++ b/src/components/overlay_mask/overlay_mask.tsx @@ -109,19 +109,18 @@ export const EuiOverlayMask: FunctionComponent = ({ }, [className, headerZindexLocation]); useEffect(() => { - if (!overlayMaskNode.current || !onClick) return; const portalTarget = overlayMaskNode.current; + if (!portalTarget || !onClick) return; + const listener = (e: Event) => { - if (e.target === overlayMaskNode.current) { + if (e.target === portalTarget) { onClick(); } }; - overlayMaskNode.current.addEventListener('click', listener); + portalTarget.addEventListener('click', listener); return () => { - if (portalTarget && onClick) { - portalTarget.removeEventListener('click', listener); - } + portalTarget.removeEventListener('click', listener); }; }, [onClick]); diff --git a/src/components/portal/portal.tsx b/src/components/portal/portal.tsx index 067d7517f94..162a18a7c2c 100644 --- a/src/components/portal/portal.tsx +++ b/src/components/portal/portal.tsx @@ -25,18 +25,18 @@ export const insertPositions: InsertPositionsMap = { before: 'beforebegin', }; +type EuiPortalInsertPosition = keyof typeof insertPositions; + export const INSERT_POSITIONS: EuiPortalInsertPosition[] = keysOf( insertPositions ); -type EuiPortalInsertPosition = keyof typeof insertPositions; - export interface EuiPortalProps { /** * ReactNode to render as this component's content */ children: ReactNode; - insert?: { sibling: HTMLElement; position: EuiPortalInsertPosition }; + insert?: { sibling: HTMLElement; position: 'before' | 'after' }; portalRef?: (ref: HTMLDivElement | null) => void; } diff --git a/src/components/search_bar/query/default_syntax.ts b/src/components/search_bar/query/default_syntax.ts index d538b529575..f5579819fc0 100644 --- a/src/components/search_bar/query/default_syntax.ts +++ b/src/components/search_bar/query/default_syntax.ts @@ -331,8 +331,9 @@ const validateFieldValue = ( try { schemaField.validate(value); } catch (e) { + const message = e instanceof Error ? e.message : e; error( - `Invalid value \`${expression}\` set for field \`${field}\` - ${e.message}`, + `Invalid value \`${expression}\` set for field \`${field}\` - ${message}`, location ); } diff --git a/src/components/search_bar/search_bar.tsx b/src/components/search_bar/search_bar.tsx index 81971d2c942..0b93e45483b 100644 --- a/src/components/search_bar/search_bar.tsx +++ b/src/components/search_bar/search_bar.tsx @@ -169,7 +169,10 @@ export class EuiSearchBar extends Component { this.notifyControllingParent({ query, queryText, error: null }); this.setState({ query, queryText, error: null }); } catch (e) { - const error: Error = { name: e.name, message: e.message }; + const error: Error = + e instanceof Error + ? { name: e.name, message: e.message } + : { name: 'Unexpected error', message: String(e) }; this.notifyControllingParent({ query: null, queryText, error }); this.setState({ queryText, error }); } diff --git a/src/components/suggest/suggest_item.tsx b/src/components/suggest/suggest_item.tsx index f4de74aef99..516f42ad5e2 100644 --- a/src/components/suggest/suggest_item.tsx +++ b/src/components/suggest/suggest_item.tsx @@ -173,7 +173,7 @@ export const EuiSuggestItem: FunctionComponent = ({ diff --git a/wiki/component-development.md b/wiki/component-development.md index 57600a31d60..d64f2ca04a4 100644 --- a/wiki/component-development.md +++ b/wiki/component-development.md @@ -53,7 +53,7 @@ Refer to the [testing guide](testing.md) for guidelines on writing and designing Refer to the [Cypress testing guide](cypress-testing.md) for guidelines on when and how to write Cypress tests. -Refer to the [automated accessibility testing guide](automated-accessibility-testing.md) for info more info on those. +Refer to the [automated accessibility testing guide](automated-accessibility-testing.md) for more info on those. ### Testing dev features in local Kibana @@ -97,6 +97,12 @@ Refer to the [SASS page][sass] of our documentation site for a guide to writing ## TypeScript definitions +### Generated props docs + +We use [react-docgen-typescript](https://github.com/styleguidist/react-docgen-typescript/) combined with some custom [props filters](../scripts/babel/react-docgen-typescript.js) to automatically generate our Props tab/table from our Typescript component types. + +> :warning: [react-docgen-typescript currently has a bug](https://github.com/styleguidist/react-docgen-typescript/issues/395) that does not correctly generate props for all components if a file has multiple components that set a `displayName`. To avoid this bug and broken props tables, keep your component files atomic / limited to 1 major component per file. + ### Pass-through props Many of our components use `rest parameters` and the `spread` operator to pass props through to an underlying DOM element. In those instances the component's TypeScript definition needs to properly include the target DOM element's props. @@ -130,10 +136,10 @@ interface FooProps extends DetailedHTMLProps