Upgrade Typescript to 4.5.3 to match Kibana #5591
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -172,7 +172,7 @@ export interface Criteria<T> { | |
| */ | ||
| sort?: { | ||
| field: keyof T; | ||
| direction: 'asc' | 'desc'; | ||
| }; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,6 +110,82 @@ export interface EuiButtonProps extends EuiButtonContentProps, CommonProps { | |
| style?: CSSProperties; | ||
| } | ||
|
|
||
| export type EuiButtonPropsForAnchor = PropsForAnchor< | ||
| EuiButtonProps, | ||
| { | ||
| buttonRef?: Ref<HTMLAnchorElement>; | ||
| } | ||
| >; | ||
|
|
||
| export type EuiButtonPropsForButton = PropsForButton< | ||
| EuiButtonProps, | ||
| { | ||
| buttonRef?: Ref<HTMLButtonElement>; | ||
| } | ||
| >; | ||
|
|
||
| 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<Props> = ({ | ||
| 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<HTMLButtonElement>['type']; | ||
| target?: string; | ||
| } = {}; | ||
|
|
||
| if (href && !buttonIsDisabled) { | ||
| relObj.href = href; | ||
| relObj.rel = getSecureRelForTarget({ href, target, rel }); | ||
| relObj.target = target; | ||
| } else { | ||
| relObj.type = type as ButtonHTMLAttributes<HTMLButtonElement>['type']; | ||
| } | ||
|
|
||
| return ( | ||
| <EuiButtonDisplay | ||
| element={element} | ||
| baseClassName="euiButton" | ||
| ref={buttonRef} | ||
| {...elementProps} | ||
| {...relObj} | ||
| {...rest} | ||
| /> | ||
| ); | ||
| }; | ||
| EuiButton.displayName = 'EuiButton'; | ||
|
Contributor
Author
There was a problem hiding this comment. The rundown on this "fix" for EuiButton's props tables - without dede73b, EuiButton's props table errors with
This was the only component with this issue that I saw while I was checking every docs page. EuiButtonEmpty, EuiButtonIcon, and EuiButtonGroup did not have this issue. EuiSkipLink had almost the exact same typing (ExclusiveUnion for anchor or button refs) and did not have this issue - a heavy hint that the problem did not lie with the actual types of EuiButton. Eventually, thanks to Chandler's SUPER helpful tips earlier on how to debug I could have moved EuiButtonDisplay out to its own EDIT: Aha, after digging more through react-docgen-typescripts, it looks like this is a known issue on their end that was introduced in 2.1.0: styleguidist/react-docgen-typescript#395 @chandlerprall any thoughts on whether we should document this limitation for now until react-docgen-typescript has a fix?
Contributor
Author
There was a problem hiding this comment. I updated our component development wiki: 97a5cd8 With all that, I think this PR is ready to go if we're OK with all of the above type changes, and if someone else could take a quick second QA pass through this PR's staging server to check that there are no other obviously broken prop tables.
Contributor
There was a problem hiding this comment. Ouch, that's a rough bug. Thanks for updating the wiki page, should be enough to help track this and point people to if/when needed. |
||
|
|
||
| export type EuiButtonDisplayProps = EuiButtonProps & | ||
| HTMLAttributes<HTMLElement> & { | ||
| /** | ||
|
|
@@ -123,12 +199,13 @@ export type EuiButtonDisplayProps = EuiButtonProps & | |
| }; | ||
|
|
||
| /** | ||
| * 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. | ||
| */ | ||
| export const EuiButtonDisplay = forwardRef<HTMLElement, EuiButtonDisplayProps>( | ||
| ( | ||
| { | ||
| element = 'button', | ||
|
|
@@ -218,77 +295,4 @@ const EuiButtonDisplay = forwardRef<HTMLElement, EuiButtonDisplayProps>( | |
| ); | ||
| } | ||
| ); | ||
| EuiButtonDisplay.displayName = 'EuiButtonDisplay'; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -169,7 +169,10 @@ export class EuiSearchBar extends Component<EuiSearchBarProps, State> { | |
| this.notifyControllingParent({ query, queryText, error: null }); | ||
| this.setState({ query, queryText, error: null }); | ||
| } catch (e) { | ||
| const error: Error = | ||
| e instanceof Error | ||
| ? { name: e.name, message: e.message } | ||
| : { name: 'Unexpected error', message: String(e) }; | ||
|
Comment on lines
+172
to
+175
Contributor
Author
There was a problem hiding this comment. Obligatory https://stackoverflow.com/a/68257472/4294462: In TS 4.4+ caught errors default to
Contributor
There was a problem hiding this comment. Maybe check for
Contributor
Author
There was a problem hiding this comment. hm, I'd be more tempted to
Contributor
There was a problem hiding this comment. Probably too engineery at that point, and leaving as-is makes more sense? 🙂
Contributor
Author
There was a problem hiding this comment. Fair enough! We can keep it in our back pockets for later if we add more try/catches |
||
| this.notifyControllingParent({ query: null, queryText, error }); | ||
| this.setState({ queryText, error }); | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.