Typescript index pattern field editor #63495
Conversation
mattkime
changed the title
|
Pinging @elastic/kibana-app-arch (Team:AppArch) |
mattkime
added
Feature:Data Views
mattkime
added
the
release_note:skip
| @@ -609,7 +609,7 @@ export class Plugin implements Plugin_2<PluginSetup, PluginStart> { | |||
| // (undocumented) | |||
| setup(core: CoreSetup, { usageCollection }: DataPluginSetupDependencies): { | |||
| fieldFormats: { | |||
| register: (customFieldFormat: import("../common").FieldFormatInstanceType) => number; | |||
| register: (customFieldFormat: import("../public").FieldFormatInstanceType) => number; | |||
There was a problem hiding this comment.
I'm uncertain why this change appears since I'm unable to find a corresponding code change. Perhaps there's an indirect change.
There was a problem hiding this comment.
Code changes LGTM. Below added few optional nits/questions.
I think I counted 62 .md files that are autogenerated in this PR, good that you raised this at our team sync, the autogenerated .md files seem indeed to be out of control, would be great to think about ways to reduce noise from them going forward.
| @@ -793,10 +838,31 @@ exports[`FieldEditor should show deprecated lang warning 1`] = ` | |||
| isVisible={false} | |||
| /> | |||
| <scripting-warning-callOut | |||
| docLinksScriptedFields={ | |||
There was a problem hiding this comment.
In this snapshot a number of times snapshot of docLinksScriptedFields is take, is that something we want to save?
| > { | ||
| static formatId = 'duration'; | ||
| state = { | ||
| ...defaultState, |
There was a problem hiding this comment.
Is it safe to use defaultState here? I'm wondering if the state of parent class could change in the future. Maybe it is safer to keep
this.state.sampleInputs = [-123, 1, 12, 123, 658, 1988, 3857, 123292, 923528271];
this.state.hasDecimalError = false;There was a problem hiding this comment.
I'm thinking about this from the perspective of "is there the potential for unwanted shared state?" Since instances of the classes will be created I think the answer is no. Yes, its possible that code might be shared in an unwanted manner somewhere down the line but 🤷
| Array [ | ||
| Object { | ||
| "content": <ScriptingSyntax | ||
| docLinksScriptedFields={ |
There was a problem hiding this comment.
Maybe in this file, too, we can stub docLinksScriptedFields as empty object {}.
| // @ts-ignore | ||
| field[fieldName] = value; |
There was a problem hiding this comment.
Could we avoid @ts-ignore here, maybe below would work
| // @ts-ignore | |
| field[fieldName] = value; | |
| (field as any)[fieldName] = value; |
|
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* Typescript index pattern field editor
Summary
Convert index pattern field editor to typescript
Note: This code could really use a larger refactoring as objects are created and modified in some very typescript unfriendly ways. I would prefer to track these items in a tech debt ticket so index pattern management can be migrated to the new platform in 7.8.
Part of #51322
Checklist
Delete any items that are not applicable to this PR.
[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support[ ] Documentation was added for features that require explanation or tutorials[ ] This was checked for keyboard-only and screenreader accessibility[ ] This renders correctly on smaller devices using a responsive layout. (You can test this in your browser[ ] This was checked for cross-browser compatibility, including a check against IE11For maintainers
[ ] This was checked for breaking API changes and was labeled appropriately