[Meta] Migrate plugins to TS references for es_ui CODEOWNERS plugins #89321

TinaHeiligers · 2021-01-26T16:25:33Z

Summary

Now that the Kibana build system supports using TypeScript project references (#46773), the es_ui team need help migrating their plugins. This is part of the overall effort tracked at #80508.

plugin	migration status
cross_cluster_replication	✅
index_lifecycle_management	✅
grokdebugger	✅
index_management	✅
license_management	✅
painless_lab	✅
remote_clusters	✅
rollup	✅
snapshot_restore	✅
upgrade_assistant	✅
watcher	✅
ingest_pipelines	✅

Migration order:

plugins ready to migrate (no blockers):
~~- watcher Migrates watcher to a TS project ref #89622~~
~~- snapshot_restore TS project refs: Migrates snapshot_restore to a TS Project #89653~~
~~- painless_lab Converts painlessLab to a TS project reference #89626~~
~~- license_management~~ Migrates license_management to a TS project ref #89472
~~- grokdebugger~~ TS project refs: Migrates grokdebugger #89652
~~- ingest_pipelines~~ Migrates ingest_pipelines to a TS project ref #89505
~~- index_management~~ Migrates index_management & runtime_fields to TS project refs #89809 (Circular dependency removed)
~~- upgrade_assistant~~ -> (Ts project refs: Migrates Upgrade Assistant to a TS project #89949)
plugins blocked by circular dependencies
~~- index_management Migrates index_management & runtime_fields to TS project refs #89809~~
- index_lifecycle_management (depends on index_management)
- remote_clusters (depends on index_management)
- rollup (depends on index_pattern_management Migrate indexPatternManagement to TS project ref #89759 & index_management)
- cross_cluster_replication (depends on remote_clusters & index_management)

Blockers (if any)

Blockers: ⚠️ Circular dependency between infra <--> apm <--> infra for
cross_cluster_replication, index_lifecycle_management, index_management, remote_clusters, rollup

Acceptance criteria

The telemetry-related plugins can be referred to using a project reference, which means each...

has a tsconfig.json file which conforms to the documented form
references the dependencies
emits type definitions for its public API
compiles in tsc's build mode

Related information

The text was updated successfully, but these errors were encountered:

TinaHeiligers · 2021-01-26T18:13:39Z

Blocked plugins and dependencies

index_management (5) (DONE)

Dependencies

~~- [ ] apm (#81003)~~

indexPatternManagement (Migrate indexPatternManagement to TS project ref #89759)
~~- [x] infra~~
licenseManagement
maps
mapsLegacy
mapsOss
~~- [ ] ml~~
~~- [ ] observability~~

index_lifecycle_management (12)

Dependencies

~~- [ ] apm (#81003)~~

remote_clusters (12)

Dependencies

~~- [ ] apm (#81003)~~

rollup (12)

Dependencies

~~- [ ] apm (#81003)~~

cross_cluster_replication (13)

Dependencies

~~- [ ] apm (#81003)~~

TinaHeiligers · 2021-01-26T23:54:39Z

@restrry the watcher plugin has a plain json file that's being imported in a few places. For example:

// x-pack/plugins/watcher/tests_client_integration
import defaultWatchJson from '../public/application/models/watch/default_watch.json';

With the following tsconfig.json:

//x-pack/plugins/watcher/tsconfig.json
{
  "extends": "../../../tsconfig.base.json",
  "compilerOptions": {
    "composite": true,
    "outDir": "./target/types",
    "emitDeclarationOnly": true,
    "declaration": true,
    "declarationMap": true
  },
  "include": [
    "common/**/*",
    "public/**/**/**/*",
    "server/**/*",
    "tests_client_integration/**/*",
    "__fixtures__/**/*",
    "../../typings/**/*"
  ],
  "references": [
    { "path": "../../../src/core/tsconfig.json" },
    { "path": "../licensing/tsconfig.json" },
    { "path": "../features/tsconfig.json" },
    { "path": "../../../src/plugins/home/tsconfig.json"},
    { "path": "../../../src/plugins/charts/tsconfig.json"},
    { "path": "../../../src/plugins/data/tsconfig.json"},
    { "path": "../../../src/plugins/management/tsconfig.json"},
    { "path": "../../../src/plugins/es_ui_shared/tsconfig.json"},
    { "path": "../../../src/plugins/kibana_react/tsconfig.json"},
  ]
}

I get the following error that I can't seem to get around:

➜  kibana git:(watcher-to-ts-ref) ✗ ./node_modules/.bin/tsc -b x-pack/plugins/watcher
x-pack/plugins/watcher/tests_client_integration/watch_create_json.test.ts:11:30 - error TS6307: File '/Users/christianeheiligers/Projects/kibana/x-pack/plugins/watcher/public/application/models/watch/default_watch.json' is not listed within the file list of project '/Users/christianeheiligers/Projects/kibana/x-pack/plugins/watcher/tsconfig.json'. Projects must list all files or use an 'include' pattern.

11 import defaultWatchJson from '../public/application/models/watch/default_watch.json';
                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Found 1 error.

Please forgive me if you've answered this before but how would one get around this?
I'm trying to resolve the issue by creating a new file and converting the json object to a js const. There are a few failing tests now though and I'm not sure how much we should be fiddling with the es_ui team's code or if I can fix them.

TinaHeiligers · 2021-01-27T15:23:31Z

@cjcenizal perhaps you can help me out with the watcher migration issue above? The TL;DR is that defaultWatchJson is declared in a .json file and typescript complains. When I change the defaultWatchJson to a js object, there are a few failing unit tests and the behavior is not the same. I'm not familiar with the watcher code and don't want to 'mess' with it too much. Thanks!

cjcenizal · 2021-01-27T18:17:34Z

@TinaHeiligers Thanks for helping us with this and for the ping! I'll take a look at Watcher. In the meantime, do you want to move it to the bottom of your list so you get to it last? I might also suggest prioritizing license management, since according to the meta issue it's blocking ML.

TinaHeiligers · 2021-01-27T18:24:12Z

@cjcenizal thank you for taking a look at watcher!

I'll prioritize license_management and will ping in this issue if there are similar problems.

In the mean time, we need to remove the dependency between feet and index_management.
index_management is a low level plugin used by many other plugins and ATM, it depends on fleet. The only reason that there is a dependency on fleet is to generate a link to the fleet app.

Could someone from the @elastic/es-ui team either look into removing that dependency as a priority?
Thanks!

cjcenizal · 2021-01-27T20:40:19Z

@TinaHeiligers

we need to remove the dependency between fleet and index_management.

Can you help me understand this need?

The only reason that there is a dependency on fleet is to generate a link to the fleet app

Are you aware of an alternative way to do this? If fleet isn't enabled, we don't want to render a link to it. What's the correct way to assert against whether it's enabled or not without declaring it an optional dependency?

TinaHeiligers · 2021-01-27T21:02:35Z

@cjcenizal, @restrry asked me to reach out about removing the dependency on fleet.
Migrating index_management to a TS project is blocked by a circular dependency between infra and apm.
The dependency chain is: index_management --> fleet --> infra --> apm
If we can remove the dependency on fleet, we (hopefully) won't need to wait for the circular dependency to be resolved (from my understanding of the request but I might be wrong here)

As for alternative ways of checking if fleet is enabled, is there a config setting you can check?

cjcenizal · 2021-01-27T22:07:34Z

@TinaHeiligers Thanks for explaining. I asked around and it sounds like we're following best practices in index_management. You mentioned the circular dependency lies between infra and apm. I think we should focus on solving that problem directly rather than trying to work around the downstream effects.

TinaHeiligers · 2021-01-28T22:00:01Z

Blockers for migrating upgrade_assistant:
~~1. Find an alternative for reading the current version~~ merged

// x-pack/plugins/upgrade_assistant/common/version.ts
import pkg from '../../../../package.json' // we're importing kibana/package.json

~~2. Find a way to reference xpackMocks:~~ Resolved with duplicating the mock used in one particular test.

~~3. Convert AssistanceData from .json to .ts:~~

~~4. Convert fakeDeprecations from .json to .ts:~~

TIL: one needs to explicitly declare .json in the tsconfig.json as follows:

// tsconfig.json
...
  },
  "include": [
    "common/**/*",
    // have to declare *.json explicitly due to https://github.com/microsoft/TypeScript/issues/25636
    "public/**/*.json",
    "server/**/*"
  ],
...

cc @elastic/es-ui

@elastic/kibana-core Does anyone have advice on how to handle 1 & 2? I can get around 3 & 4.

cjcenizal · 2021-01-28T22:30:01Z

@alisonelizabeth Could you help Tina out with 1 and 2 above?

alisonelizabeth · 2021-01-29T01:16:28Z

I can look into this on Friday.

mshustov · 2021-01-29T13:15:32Z

@TinaHeiligers what's the problem with json files? The same as here?

kibana/x-pack/plugins/actions/tsconfig.json

Lines 12 to 13 in 8b1a228

    
           // have to declare *.json explicitly due to https://github.com/microsoft/TypeScript/issues/25636 
        
           "server/**/*.json",

TinaHeiligers · 2021-01-29T14:38:38Z

@restrry oh wow, I didn't know we could do that!
Thanks

alisonelizabeth · 2021-01-29T16:35:08Z

@TinaHeiligers Just a quick update on UA - I can read the current version from PluginInitializerContext.env.packageInfo.version instead of package.json. I will also handle xpackMocks. I'll tag you when I have a PR up.

TinaHeiligers mentioned this issue Jan 26, 2021

[Meta] Migrate Kibana plugins to TS project references #80508

Closed

TinaHeiligers added this to 7.12 in kibana-core [DEPRECATED] Jan 26, 2021

TinaHeiligers added chore release_note:skip Skip the PR/issue when compiling release notes v7.12.0 v8.0.0 labels Jan 26, 2021

TinaHeiligers mentioned this issue Jan 27, 2021

Migrates license_management to a TS project ref #89472

Merged

cjcenizal mentioned this issue Jan 27, 2021

Convert default_watch.json to a JS object in order to avoid TS complaints #89488

Merged

TinaHeiligers mentioned this issue Jan 27, 2021

Migrates ingest_pipelines to a TS project ref #89505

Merged

cjcenizal closed this as completed in #89488 Jan 28, 2021

kibana-core [DEPRECATED] automation moved this from 7.12 to Done (7.12) Jan 28, 2021

TinaHeiligers reopened this Jan 28, 2021

This was referenced Jan 28, 2021

Migrates watcher to a TS project ref #89622

Merged

Converts painlessLab to a TS project reference #89626

Merged

This was referenced Jan 28, 2021

TS project refs: Migrates grokdebugger #89652

Merged

TS project refs: Migrates snapshot_restore to a TS Project #89653

Merged

TinaHeiligers closed this as completed in #89622 Jan 29, 2021

TinaHeiligers reopened this Jan 29, 2021

TinaHeiligers moved this from Done (7.12) to In Progress in kibana-core [DEPRECATED] Jan 29, 2021

alisonelizabeth mentioned this issue Jan 29, 2021

[Upgrade Assistant] Use kibana version value from core context #89774

Merged

This was referenced Jan 29, 2021

Migrates index_management & runtime_fields to TS project refs #89809

Merged

Ts project refs: Migrates Upgrade Assistant to a TS project #89949

Merged

TinaHeiligers mentioned this issue Feb 9, 2021

TS project refs: Converts rollup, remoteClusters, crossClusterReplication, indexLifecycleManagement to a TS project refs #90713

Merged

TinaHeiligers closed this as completed in #90713 Feb 9, 2021

kibana-core [DEPRECATED] automation moved this from In Progress to Done (7.12) Feb 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Meta] Migrate plugins to TS references for es_ui CODEOWNERS plugins #89321

[Meta] Migrate plugins to TS references for es_ui CODEOWNERS plugins #89321

TinaHeiligers commented Jan 26, 2021 •

edited

TinaHeiligers commented Jan 26, 2021 •

edited

Dependencies

Dependencies

Dependencies

Dependencies

Dependencies

TinaHeiligers commented Jan 26, 2021 •

edited

TinaHeiligers commented Jan 27, 2021

cjcenizal commented Jan 27, 2021

TinaHeiligers commented Jan 27, 2021

cjcenizal commented Jan 27, 2021

TinaHeiligers commented Jan 27, 2021 •

edited

cjcenizal commented Jan 27, 2021

TinaHeiligers commented Jan 28, 2021 •

edited

cjcenizal commented Jan 28, 2021

alisonelizabeth commented Jan 29, 2021

mshustov commented Jan 29, 2021 •

edited

TinaHeiligers commented Jan 29, 2021

alisonelizabeth commented Jan 29, 2021

[Meta] Migrate plugins to TS references for es_ui CODEOWNERS plugins #89321

[Meta] Migrate plugins to TS references for es_ui CODEOWNERS plugins #89321

Comments

TinaHeiligers commented Jan 26, 2021 • edited

Summary

Blockers (if any)

Acceptance criteria

Related information

TinaHeiligers commented Jan 26, 2021 • edited

Blocked plugins and dependencies

Dependencies

Dependencies

Dependencies

Dependencies

Dependencies

TinaHeiligers commented Jan 26, 2021 • edited

TinaHeiligers commented Jan 27, 2021

cjcenizal commented Jan 27, 2021

TinaHeiligers commented Jan 27, 2021

cjcenizal commented Jan 27, 2021

TinaHeiligers commented Jan 27, 2021 • edited

cjcenizal commented Jan 27, 2021

TinaHeiligers commented Jan 28, 2021 • edited

cjcenizal commented Jan 28, 2021

alisonelizabeth commented Jan 29, 2021

mshustov commented Jan 29, 2021 • edited

TinaHeiligers commented Jan 29, 2021

alisonelizabeth commented Jan 29, 2021

TinaHeiligers commented Jan 26, 2021 •

edited

TinaHeiligers commented Jan 26, 2021 •

edited

TinaHeiligers commented Jan 26, 2021 •

edited

TinaHeiligers commented Jan 27, 2021 •

edited

TinaHeiligers commented Jan 28, 2021 •

edited

mshustov commented Jan 29, 2021 •

edited