-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Migrates index_management & runtime_fields to TS project refs #89809
Conversation
@elasticmachine merge upstream |
951e8fe
to
c53a53d
Compare
Aren't the Before and After values reversed? |
@restrry The updated values now reflect that after the migration, we have fewer Files and Lines than before. |
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
💚 Build SucceededMetrics [docs]
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this 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); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 :)
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 ofruntime_fields
to a TS project ref.