Kibana: Switch to typescript project references and incremental builds

Created on 27 Sep 2019 · 19Comments · Source: elastic/kibana

Developing Typescript with VSCode is currently frustratingly slow:

Opening files in another one of Kibana's 24 independant typescript projects sometimes takes > 60s "initializing TS language features"
cmd+clicking references within some projects takes > 60s (in my experience test seems to be the slowest)
Not all type errors show up under VSCode's "problems"
Running a complete typecheck takes > 60s which slows down the development cycle when doing large type refactors

I believe using [typescript references](https://www.typescriptlang.org/docs/handbook/project-references.html and incremental builds we should be able to make the build time much faster, have type errors show up without having to run scripts/type_check and improve type lookups and navigation in VSCode.

The first iteration https://github.com/elastic/kibana/pull/72280

Steps

[x] src/test_utils to TS project https://github.com/elastic/kibana/pull/76082
[x] src/core to TS project https://github.com/elastic/kibana/pull/76785
[x] src/plugins/kibana_utils to TS project https://github.com/elastic/kibana/pull/78440
[x] x-pack/plugins/licensing to TS project https://github.com/elastic/kibana/pull/78440
[x] document migration path for other plugins https://github.com/elastic/kibana/pull/78586
[x] create a tool to identify plugin deps https://github.com/elastic/kibana/issues/79343
[ ] plugins migrate to TS project refs. meta issue https://github.com/elastic/kibana/issues/80508

Development Core Operations v7.10.0

Source

rudolf

👍2

Most helpful comment

PR extracting src/test_utils into TS project https://github.com/elastic/kibana/pull/76082
Next steps:

introduce project references for platform code under src/core
introduce project references for src/plugins/kibana_utils
document migration path for solution teams

src/core migration

To migrate to TS project references, we need to comply with several requirements:

[x] src/core projects imports only from explicitly defined TS projects (src/test_utils)
[ ] src/core compiles all the TS files it imports

There are a few blockers at the moment:

CLI

We can fix this by changing import to *js file temporarily until we remove this dependency.

Data plugin

Already has issue https://github.com/elastic/kibana/issues/55485

Legacy Utils

Migrate to KP stream helpers https://github.com/elastic/kibana/pull/76397

JS deps

They aren't hard requirements, but we should remove them eventually:

Legacy Config

Legacy server

Legacy plugins

[ ] https://github.com/elastic/kibana/blob/master/src/core/server/legacy/plugins/find_legacy_plugin_specs.ts#L23-L29

tracked as

yarn add -D dependency-cruiser
./node_modules/.bin/dependency-cruiser --init
rm deps.txt && ./node_modules/.bin/dependency-cruiser src/core/ -x "^(node_modules|packages)" -T text | grep -v '→ *src\/core*' > deps.txt

restrry on 1 Sep 2020

🎉1 👍1

All 19 comments

Pinging @elastic/kibana-operations

elasticmachine on 27 Sep 2019

I've tried to do this a couple times but it wasn't very easy, though I agree it's worth putting real effort into.

spalger on 1 Oct 2019

This is unrelated to Kibana (I was Googling for Project References information), but when I try running tsc --build --incremental --tsBuildInfoFile ./.buildinfo where --build enabled the new Project References features, it complains error TS5072: Unknown build option '--tsBuildInfoFile'.

I'm not sure, but I think that incremental builds are a default feature of Project References (--build) if I read that that doc page correctly.

EDIT: Ah, yep! This is what I see in watch mode with --build --watch:

[7:43:06 PM] File change detected. Starting incremental compilation...

Note the "incremental" part. So, yeah, it is incremental by default. Cool!

trusktr on 12 May 2020

I did a quick (and dirty) spike to get incremental builds working for Kibana: https://github.com/elastic/kibana/pull/46772

rudolf on 12 May 2020

@rudolf Could you measure the effect with TS built-in diagnostic tools https://github.com/microsoft/TypeScript/wiki/Performance#investigating-issues?

restrry on 9 Jul 2020

🎉1

@rudolf Could you measure the effect with TS built-in diagnostic tools

Good idea!

rudolf on 9 Jul 2020

Pinging @elastic/kibana-platform (Team:Platform)

elasticmachine on 9 Jul 2020

Combined this issue with https://github.com/elastic/kibana/pull/72280

jinmu03 on 14 Aug 2020

All tests done on https://github.com/elastic/kibana/pull/73924 with TS v4.0.2
TS provides several options to improve perf:

Incremental builds

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-4.html#faster-subsequent-builds-with-the---incremental-flag
Can be used with —noEmit flag.
Generates .tsbuildinfo fail to cache dependency tree.
How it affects *type check time of OSS project:

incremental: false first pass 33.9s
incremental: false second pass 33.8s

Files:             5779
Lines:            783785
Nodes:           2632074
Identifiers:         897987
Symbols:          1750565
Types:            597330
Instantiations:       2072774
Memory used:        1675133K
Assignability cache size:  255152
Identity cache size:     14662
Subtype cache size:      21525
Strict subtype cache size:  18711
I/O Read time:        0.31s
Parse time:          3.19s
ResolveTypeReference time:  0.06s
ResolveModule time:      1.73s
Program time:         5.88s
Bind time:          1.58s
Check time:         26.33s
printTime time:        0.00s
Emit time:          0.00s
Total time:         33.79s

incremental: true first pass: 45s

Files:             5779
Lines:            783785
Nodes:           2632074
Identifiers:         897987
Symbols:          1751312
Types:            597411
Instantiations:       2074247
Memory used:        1761445K
Assignability cache size:  255108
Identity cache size:     14662
Subtype cache size:      21512
Strict subtype cache size:  18715
I/O Read time:        0.36s
Parse time:          3.13s
ResolveTypeReference time:  0.04s
ResolveModule time:      1.72s
Program time:         5.93s
Bind time:          1.61s
Check time:         24.13s
transformTime time:     11.41s
commentTime time:       0.30s
printTime time:       13.34s
Emit time:          13.38s
I/O Write time:        0.02s
Total time:         45.05s

incremental: true second pass: 7.4s

Files:            5779
Lines:           783785
Nodes:           2632074
Identifiers:        897987
Symbols:          485640
Types:             76
Instantiations:         0
Memory used:        798209K
Assignability cache size:    0
Identity cache size:       0
Subtype cache size:       0
Strict subtype cache size:    0
I/O Read time:        0.32s
Parse time:         3.07s
ResolveTypeReference time:  0.05s
ResolveModule time:     1.61s
Program time:        5.78s
Bind time:          1.65s
Total time:         7.43s

incremental: true an exported type changed: 45.6s
incremental: true an internal type changed 42.6s

How it affects type build time of the OSS project:

incremental: false first pass: 20.8s
incremental: true first pass: 24.5s
incremental: true second pass: 4.6s
incremental: true an internal type changed 33.6s

We can conclude that it speeds up type check and build time if code hasn’t changed. Which is not the case for actively developed projects as Kibana. We could reduce scopes and make projects more focused - make every plugin a separate TS project at least for x-pack plugins, extract oss & x-pack tests in separate ts projects as well. @elastic/kibana-operations Do you envision any problems with this approach?

Project references

https://www.typescriptlang.org/docs/handbook/project-references.html
It turned out that we cannot use project references without building project types for referenced projects (we need to emit at least their type declarations). We hoped that TS from v4 allows this use case with —-noEmit flag but turned out it’s not possible by design https://github.com/microsoft/TypeScript/pull/39122/files#diff-27762b0db782b18c2ddc01669e7dd54fR33
I need to take another stab at this work started in https://github.com/elastic/kibana/pull/72280

restrry on 21 Aug 2020

❤2

Thank you so much for doing this research.

We could reduce scopes and make projects more focused - make every plugin a separate TS project at least for x-pack plugins, extract oss & x-pack tests in separate ts projects as well. @elastic/kibana-operations Do you envision any problems with this approach?

This is definitely what I hope we will do, though I don't think it will be easy.

spalger on 21 Aug 2020

Ok, I'm going to start with a single plugin and a test project to see what problems we discover along the way.

restrry on 21 Aug 2020

👍1

Today I had a stab at composite project integration Kibana repo.

To make them work we should:

build types for every project separately
TS projects should form direct acyclic graph. Circular refs between project aren't allowed https://github.com/microsoft/TypeScript/issues/33685

It requires starting from a leaf project with no dependencies. It would seem that the Kibana platform is an apparent candidate for this role. But it's not that simple, unfortunately:

The platform depends on the data plugin on the server-side. Shouldn't we remove this dependency? Extracting KQL into a separate package, maybe? @elastic/kibana-platform
The platform depends on cli/cluster_manager. Can it be moved under src/core/server? @elastic/kibana-operations
There are several dependencies on src/legacy. Either we remove them or move them under src/core.
The platform depends on legacy/server/kbn_server which has deps on several plugins. These deps can likely be removed since there are no more legacy server consumers in the solutions code.
The platform tests depend on test_utils/kbn_server which likewise needs moving under the platform folder.

src/test_utils is another small candidate to move to TS projects, but we need to remove deps on the platform to make it happen.

restrry on 24 Aug 2020

The platform depends on the data plugin on the server-side. Shouldn't we remove this dependency? Extracting KQL into a separate package, maybe? @elastic/kibana-platform

There was a discussion about this topic when the core->data dependency was actually introduced. Don't exactly remember what the plan was though. @rudolf @elastic/kibana-app-arch does someone remember what our options were?

The platform depends on cli/cluster_manager. Can it be moved under src/core/server?

Let's see what operations have to say about this one, but moving at least the typedefs to core SGTM.

There are several dependencies on src/legacy. Either we remove them or move them under src/core

Lots of them should disappear with the legacy plugin discovery removal + legacy service. Most of the ones remaining should be imports of src/legacy/utils/streams from core. This package should probably move to core until we decide exactly what to do with it.

The platform tests depend on test_utils/kbn_server which likewise needs moving under the platform folder.

src/test_utils is another small candidate to move to TS projects, but we need to remove deps on the platform to make it happen.

Would have hoped that creating test_utils/kbn_server project would be easy, however there are bidirectional dependencies.

From a (very) quick look, it seems to be only from

src/test_utils/public/http_test_setup.ts
src/test_utils/kbn_server.ts

However src/test_utils/kbn_server.ts also imports from ../legacy/... so not sure of the difficulty to move this to core.

pgayvallet on 24 Aug 2020

There was a discussion about this topic when the core->data dependency was actually introduced. Don't exactly remember what the plan was though. @rudolf @elastic/kibana-app-arch does someone remember what our options were?

We opened an issue for this with a high-level plan forward here: https://github.com/elastic/kibana/issues/55485
Basically, the proposal is to allow arbitrary query predicates to be passed into the find API. I believe @kobelb has also found that adding this would allow for some perf improvements to some Fleet endpoints that currently need KQL-like functionality.

The platform depends on cli/cluster_manager. Can it be moved under src/core/server?

Let's see what operations have to say about this one, but moving at least the typedefs to core SGTM.

The platform tests depend on test_utils/kbn_server which likewise needs moving under the platform folder.

I wonder if we should just re-write this utility in Core and adapt the existing usages of it to this new test harness. I only see 18 usages of it, so I don't think it will be too difficult.

joshdover on 24 Aug 2020

The platform depends on cli/cluster_manager. Can it be moved under src/core/server? @elastic/kibana-operations

My instinct is to decouple the core from the CLI rather than couple them further. It might be challenging because the CLI requires some config, so maybe we need to move it into core temporarily until we can decouple the CLI and keep it indenedent.

spalger on 24 Aug 2020

Basically, the proposal is to allow arbitrary query predicates to be passed into the find API

My one concern is that allowing consumers to directly write the query predicates ends up leaking the way we map from a saved-object to the Elasticsearch document which represents it. This is the one benefit that we get from KQL, it's structured in such a manner that we can rewrite the query. For example, a consumer of the find API shouldn't need to know that we put everything under attributes in a field which matches the type. You can't just write the following to look for a dashboard with a specific :

"query": {
    "term": {
      "attributes.description": {
        "value": "foo"
      }
    }
  }

you have to write

"query": {
    "term": {
      "dashboard.description": {
        "value": "foo"
      }
    }
  }

I believe @kobelb has also found that adding this would allow for some perf improvements to some Fleet endpoints that currently need KQL-like functionality.

FWIW, we found a way around this by allowing Fleet endpoints to build the KQL manually and not parse it from a string https://github.com/elastic/kibana/pull/75693

kobelb on 24 Aug 2020

WIP PR moving src/test_utils in to a separate project: https://github.com/restrry/kibana/pull/6/

restrry on 25 Aug 2020