Skip to content

Prepare for TS 3.7 upgrade#47794

Merged
timroes merged 2 commits intoelastic:masterfrom
timroes:ts-cleanup-misc
Oct 14, 2019
Merged

Prepare for TS 3.7 upgrade#47794
timroes merged 2 commits intoelastic:masterfrom
timroes:ts-cleanup-misc

Conversation

@timroes
Copy link
Copy Markdown
Contributor

@timroes timroes commented Oct 10, 2019

Summary

This PR addresses a couple of TS issues, that will break once upgraded to TS 3.7.0. This PR mostly contains smaller changes not really effecting any specific plugin a lot, so I randomly select a couple of TS experts for review here.

For QA: This PR does not contain any functional changes.

@timroes timroes added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.5.0 labels Oct 10, 2019
@timroes timroes requested a review from a team as a code owner October 10, 2019 07:40
@timroes timroes requested a review from a team October 10, 2019 07:40

public switchViewMode = () => {
this.setState((prevState: State) => {
this.setState<'viewMode'>((prevState: State) => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ℹ️ It complains that { viewMode } is missing loading, because I think it cannot automatically infer the generic type here, due to the if (it actually infers it to never).

map(url => url.hostname),
mapNullable(hostname => isWhitelisted(config, hostname)),
getOrElse(() => false)
getOrElse<boolean>(() => false)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ℹ️ It infers the generic type false here, and then complains, that the mapNullable produces a boolean which is not assignable to false. I think the typings of pipe are just a bit weird, that they are "checking backwards" from the last function.

}

export type AlertTypeRegistry = PublicMethodsOf<AlertTypeRegistry>;
export type AlertTypeRegistry = PublicMethodsOf<OrigAlertTypeRegistry>;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ℹ️ The usual 3.7 issue, that you cannot have an import and export with the same name (but different values) in one file anymore.

message?: any;
rowProps?: any;
cellProps?: any;
responsive?: boolean;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ℹ️ This was just used somewhere, and I honestly have no idea how this didn't fail in the current TS .

Object.keys(IndexGroup).forEach(typeKey => {
const consumerType = IndexGroup[typeKey as any] as IndexGroup;

Object.entries(IndexGroup).forEach(([typeKey, consumerType]) => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ℹ️ The as any typing complained, that any is not a valid key to the IndexGroup enum. So I actually just used the entries method here, which a/ looks nicer and b/ is perfectly type safe now.

@timroes timroes mentioned this pull request Oct 10, 2019
15 tasks
@timroes
Copy link
Copy Markdown
Contributor Author

timroes commented Oct 11, 2019

Jenkins, test this - CI failed to report

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@timroes
Copy link
Copy Markdown
Contributor Author

timroes commented Oct 14, 2019

@elasticmachine merge upstream

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@timroes
Copy link
Copy Markdown
Contributor Author

timroes commented Oct 14, 2019

Ignoring CODEOWNERS review since it only changing very minor typings in their files.

@timroes timroes merged commit 679bce1 into elastic:master Oct 14, 2019
@timroes timroes deleted the ts-cleanup-misc branch October 14, 2019 20:23
TinaHeiligers pushed a commit to TinaHeiligers/kibana that referenced this pull request Oct 14, 2019
timroes pushed a commit that referenced this pull request Oct 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes v7.5.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants