add generic typings for SavedObjectMigrationFn #63943
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,7 +26,7 @@ import { SavedObjectsMigrationLogger } from './core/migration_logger'; | |
| * | ||
| * @example | ||
| * ```typescript | ||
| * const migrateProperty: SavedObjectMigrationFn<MyUnmigratedAttributes, MyMigratedAttributes> = (doc, { log }) => { | ||
| * if(doc.attributes.someProp === null) { | ||
| * log.warn('Skipping migration'); | ||
| * } else { | ||
|
|
@@ -39,10 +39,10 @@ import { SavedObjectsMigrationLogger } from './core/migration_logger'; | |
| * | ||
| * @public | ||
| */ | ||
| export type SavedObjectMigrationFn<InputAttributes = unknown, MigratedAttributes = unknown> = ( | ||
| doc: SavedObjectUnsanitizedDoc<InputAttributes>, | ||
| context: SavedObjectMigrationContext | ||
| ) => SavedObjectUnsanitizedDoc<MigratedAttributes>; | ||
|
Comment on lines
+56
to
+59
There was a problem hiding this comment. So, my only interrogation around that is that atm migrations are all mutating and returning the input I.E export const migrateToKibana660: SavedObjectMigrationFn<any, any> = doc => {
if (!doc.attributes.hasOwnProperty('disabledFeatures')) {
doc.attributes.disabledFeatures = [];
}
return doc;
};Meaning that type InputAttrs = {
}
type OutputAttrs = {
disabledFeatures: SomeType[];
}
export const migrateToKibana660: SavedObjectMigrationFn<InputAttrs, OutputAttrs> = doc => {
if (!doc.attributes.hasOwnProperty('disabledFeatures')) {
doc.attributes.disabledFeatures = [];
}
return doc;
};Would fail to compile with something like Which is why I'm wondering if we should add the output properties on the input export type SavedObjectMigrationFn<InputAttributes = unknown, MigratedAttributes = unknown> = (
doc: SavedObjectUnsanitizedDoc<InputAttributes & MigratedAttributes>,
context: SavedObjectMigrationContext
) => SavedObjectUnsanitizedDoc<MigratedAttributes>;Ideally I would have been using Partial, The other option being to decide that migration function should properly spread / construct the resulting migrated object without returning directly the input There was a problem hiding this comment.
Given the high impact of even a small We can leave existing migrations with There was a problem hiding this comment. As a slightly more type-safe solution than There was a problem hiding this comment. SGTM, I'll keep the definition as it is right now then. |
||
|
|
||
| /** | ||
| * Migration context provided when invoking a {@link SavedObjectMigrationFn | migration handler} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,7 +14,8 @@ interface XYLayerPre77 { | |
| accessors: string[]; | ||
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| export const migrations: Record<string, SavedObjectMigrationFn<any, any>> = { | ||
|
Comment on lines
-134
to
+137
There was a problem hiding this comment. Caused by Lines 736 to 741 in 40f8222 |
||
| '7.7.0': doc => { | ||
| const newDoc = cloneDeep(doc); | ||
| if (newDoc.attributes?.visualizationType === 'lnsXY') { | ||
|
|
||
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.
we should update the example to show how to construct a copy so that the output is typed different to the input. Since this is only for type safety we don't need to make a deep copy, so it might be worth adding a comment to the example to highlight this.