Skip to content
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

Migrates index_management & runtime_fields to TS project refs #89809

Conversation

TinaHeiligers
Copy link
Contributor

@TinaHeiligers TinaHeiligers commented Jan 29, 2021

Summary

Partially addresses #89321

tsc compiler performance with running node --max-old-space-size=4096 ./node_modules/.bin/tsc -p x-pack/tsconfig.json --extendedDiagnostics --noEmit

Before
Files: 15620
Lines: 1645697

After
Files: 14711
Lines: 1600254

Updated 1 Feb 2021
Note: The PR includes the changes in #89699 and, potentially, duplicates migration of runtime_fields to a TS project ref.

@TinaHeiligers TinaHeiligers added chore v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes v7.12.0 labels Jan 29, 2021
@TinaHeiligers TinaHeiligers changed the title Hack Migrates index_management & runtime_fields to TS project refs Jan 29, 2021
@TinaHeiligers
Copy link
Contributor Author

@elasticmachine merge upstream

@TinaHeiligers TinaHeiligers force-pushed the ts-project-refs-es_ui/index_management branch from 951e8fe to c53a53d Compare January 30, 2021 14:24
@mshustov
Copy link
Contributor

Aren't the Before and After values reversed?

@TinaHeiligers
Copy link
Contributor Author

TinaHeiligers commented Feb 1, 2021

@restrry The updated values now reflect that after the migration, we have fewer Files and Lines than before.

@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

These changes look good to me! Did not test compiling locally, happy that CI is passing though.

@@ -21,4 +21,4 @@ const testBedConfig: TestBedConfig = {

const initTestBed = registerTestBed(WithAppDependencies(TemplateClone), testBedConfig);

export const setup = formSetup.bind(null, initTestBed);
export const setup: any = formSetup.bind(null, initTestBed);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, why are these needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The type isn't inferred from formSetup and the typescript project couldn't build.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah OK. It is surprising that .bind returns any! What we'd like to express is:

const a = (a: number) => {};

const b: () => ReturnType<typeof a> = a.bind(null, 1);

Not sure what else could be done, other than enabling strictBindCallApply in tsconfig, which is a breaking type check change -- but would solve this for us :)

@TinaHeiligers TinaHeiligers merged commit 9bee677 into elastic:master Feb 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants