Skip to content

fix(ui): Typescript 4 misc fixes #20703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/sentry/static/sentry/app/components/button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ const getIconMargin = ({size, hasChildren}: IconProps) => {
return size && size.endsWith('small') ? '6px' : '8px';
};

const Icon = styled('span')<IconProps>`
const Icon = styled('span')<IconProps & Omit<StyledButtonProps, 'theme'>>`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needed all the types for getFontSize and getIconMargin

display: flex;
align-items: center;
margin-right: ${getIconMargin};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export function hasNonContributingComponent(component: EventGroupComponent | und
}

export function shouldInlineComponentValue(component: EventGroupComponent) {
return component.values.every(value => !isObject(value));
return (component.values as EventGroupComponent[]).every(value => !isObject(value));
Copy link
Member Author

@scttcper scttcper Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wasn't sure on this, but it didn't like calling .every on type EventGroupComponent[] | string[] possibly because EventGroupComponent was self referencing

Copy link
Member Author

@scttcper scttcper Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typescript playground of this weird one

}

export function groupingComponentFilter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,8 @@ export const setBodyUserSelect = (nextValues: UserSelectValues): UserSelectValue
// MozUserSelect is not typed in TS
// @ts-ignore
MozUserSelect: document.body.style.MozUserSelect,
// msUserSelect is not typed in TS
// @ts-ignore
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

msUserSelect is now also missing in ts 4

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any idea why? Should we be dropping support too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep this.

I'm aware of this being a missing type in TypeScript. This was causing an issue now because of updated type definitions for user select on v4.

msUserSelect: document.body.style.msUserSelect,
webkitUserSelect: document.body.style.webkitUserSelect,
};
Expand All @@ -315,6 +317,8 @@ export const setBodyUserSelect = (nextValues: UserSelectValues): UserSelectValue
// MozUserSelect is not typed in TS
// @ts-ignore
document.body.style.MozUserSelect = nextValues.MozUserSelect || '';
// msUserSelect is not typed in TS
// @ts-ignore
document.body.style.msUserSelect = nextValues.msUserSelect || '';
document.body.style.webkitUserSelect = nextValues.webkitUserSelect || '';

Expand Down
14 changes: 0 additions & 14 deletions src/sentry/static/sentry/app/components/selectMembers/index.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import PropTypes from 'prop-types';
import React from 'react';
import debounce from 'lodash/debounce';
import styled from '@emotion/styled';
Expand All @@ -14,7 +13,6 @@ import IdBadge from 'app/components/idBadge';
import MemberListStore from 'app/stores/memberListStore';
import ProjectsStore from 'app/stores/projectsStore';
import SelectControl from 'app/components/forms/selectControl';
import SentryTypes from 'app/sentryTypes';
import TeamStore from 'app/stores/teamStore';
import Tooltip from 'app/components/tooltip';
import withApi from 'app/utils/withApi';
Expand Down Expand Up @@ -77,18 +75,6 @@ type FilterOption<T> = {
* A component that allows you to select either members and/or teams
*/
class SelectMembers extends React.Component<Props, State> {
static propTypes = {
project: SentryTypes.Project,
organization: SentryTypes.Organization,
value: PropTypes.string,
onChange: PropTypes.func.isRequired,
onInputChange: PropTypes.func,
disabled: PropTypes.bool,
styles: PropTypes.shape({
control: PropTypes.func,
}),
};

state: State = {
loading: false,
inputValue: '',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ export const fields: Record<string, Field> = {
`This can be used to modify the fingerprinting rules on the server with custom rules.
Rules follow the pattern [pattern]. [docs:Read the docs] for more information.`,
{
pattern: <code>matcher:glob -> fingerprint, values</code>,
pattern: <code>matcher:glob -&gt; fingerprint, values</code>,
docs: (
<ExternalLink href="https://docs.sentry.io/platform-redirect/?next=%2Fdata-management%2Fevent-grouping%2Fserver-side-fingerprinting%2F" />
),
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/static/sentry/app/types/alerts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export type UnsavedIssueAlertRule = {
actions: IssueAlertRuleAction[];
conditions: IssueAlertRuleCondition[];
filters: IssueAlertRuleCondition[];
environment: null | string;
environment?: null | string;
frequency: number;
name: string;
};
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/static/sentry/app/types/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1082,7 +1082,7 @@ export type NewQuery = {
createdBy?: User;

// Query and Table
query: string;
query?: string;
fields: Readonly<string[]>;
widths?: Readonly<string[]>;
orderby?: string;
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/static/sentry/app/utils/discover/eventView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,7 @@ class EventView {
const environment = this.environment as string[];

// generate event query
const eventQuery: EventQuery & LocationQuery = Object.assign(
const eventQuery = Object.assign(
Copy link
Member Author

@scttcper scttcper Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This errors in ts4

i think this change is more correct, eventQuery isn't EventQuery & LocationQuery, but instead we're casting the result from object.assign to EventQuery & LocationQuery

omit(picked, DATETIME_QUERY_STRING_KEYS),
normalizedTimeWindowParams,
{
Expand All @@ -929,7 +929,7 @@ class EventView {
per_page: DEFAULT_PER_PAGE,
query: this.query,
}
);
) as EventQuery & LocationQuery;

if (!eventQuery.sort) {
delete eventQuery.sort;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,6 @@ class ActivityContainer extends React.PureComponent<Props, State> {

return (
<Activity
noteInputId={this.state.noteInputId}
alertId={alertId}
me={me}
api={api}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ class EventDetailsContent extends AsyncComponent<Props, State> {
}
const eventReference = {...event};
if (eventReference.id) {
delete eventReference.id;
delete (eventReference as any).id;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about refactoring the spread to something like

{id:_, ...eventReference} = event

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eventReference is passed to functions that then expect Event type. Without the id it errors as well. Could change the type of Event but that also feels like a bigger change.

}
const tagKey = this.generateTagKey(tag);
const nextView = getExpandedResults(eventView, {[tagKey]: tag.value}, eventReference);
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/static/sentry/app/views/eventsV2/queryList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ class QueryList extends React.Component<Props> {
onCursor={(cursor: string, path: string, query: Query, direction: number) => {
const offset = Number(cursor.split(':')[1]);

const newQuery = {...query, cursor};
const newQuery: Query & {cursor?: string} = {...query, cursor};
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allow deletion via optional

const isPrevious = direction === -1;

if (offset <= 0 && isPrevious) {
Expand Down