Migrate inline with same sort

[rarkins]

What would you like Renovate to be able to do?

When Renovate migrates field A to field B, B should be positioned within the config exactly where A was, if possible. This will make the migrations more readable.

If you have any ideas on how this should be implemented, please tell us here.

It will require us to iterate through keys of each object and then recreate the migrated object field by field.

Is this a feature you are interested in implementing yourself?

No

[pret-a-porter]

@rarkins could you please provide some example? What exactly do you expect?

[HonkingGoose]

When Renovate migrates field A to field B, B should be positioned within the config exactly where A was, if possible. This will make the migrations more readable.

I like this a lot! It will make any "automatic migration PR" way easier to review (#1502).

[rarkins]

@pret-a-porter when we migrate a field or object today, we often delete the existing field and add the replacement field. The problem is that the replacement field is added to the existing object and so it goes "last". This means that although the migration might be a simple +/- of one line each, the deletion might occur near the top of the object and the addition near the bottom. This is not like how a human would make such a changae. If you combine a few of these together then it makes it much harder to read the diff.

Compare this before:

renovate/lib/config/migration.spec.ts

Line 20 in ef08699

const config: TestRenovateConfig = {

With this after:

renovate/lib/config/__snapshots__/migration.spec.ts.snap

Line 85 in ef08699

Object {

[pret-a-porter]

I think it can be something like this:

interface OldConfig {
    a: string;
    b: string;
    c: string;
}

interface NewConfig {
    a: string;
    d: number;
    c: string;
}

class ExampleMigration {
    private readonly order: ReadonlyArray<keyof NewConfig> = ['a', 'd', 'c'];

    protected readonly oldConfig: OldConfig;

    public constructor(oldConfig: OldConfig) {
        this.oldConfig = oldConfig;
    }

    public migrate(): NewConfig {
        const newConfig: Partial<NewConfig> = {};
        for (const property of this.order) {
            if (property === 'd') {
                newConfig[property] = 1;
            } else {
                newConfig[property] = this.oldConfig[property];
            }
        }

        return newConfig as NewConfig;
    };
}

@rarkins what do you think?

[viceice]

Our config don't enforce any orders, so you need to get the order dynamically

[pret-a-porter]

I thought we can introduce order, dynamic order can be received only if we have something like DEFAULT_CONFIG with all fields

[viceice]

There won't be a default order ever. We need to use the user defined order and use some in place change.

Eg, create new object, iterate over key-value pairs, do required migration, set update new object.

That way object key order should be retained.

Arrays can be done in similar loop.

[pret-a-porter]

So, in general looks clear what should be done, I am ready to make draft PR

[rarkins]

@pret-a-porter my starting point would be something like this:

diff --git a/lib/config/migration.ts b/lib/config/migration.ts
index 1fb49fc3a..cce1da317 100644
--- a/lib/config/migration.ts
+++ b/lib/config/migration.ts
@@ -49,7 +49,7 @@ export function migrateConfig(
         optionTypes[option.name] = option.type;
       });
     }
-    const migratedConfig = clone(config) as MigratedRenovateConfig;
+    const migratedConfig = {} as MigratedRenovateConfig;
     const depTypes = [
       'dependencies',
       'devDependencies',
@@ -60,6 +60,7 @@ export function migrateConfig(
     const { migratePresets } = getGlobalConfig();
     applyMigrations(config, migratedConfig);
     for (const [key, val] of Object.entries(config)) {
+      migratedConfig[key] = clone(val);
       if (removedOptions.includes(key)) {
         delete migratedConfig[key];
       } else if (key === 'pathRules') {

The above change results in Tests: 5 failed, 23 passed, 28 total

Today when we clone the original config right away, anything new we add gets added to the end of the migrated object. So this change idea was to only add to the migrated object key-by-key to preserve ordering.

One challenge with this is that jest snapshots re-sort keys of objects.. so we may need to do expects on Object.keys() which should return the right order we want.

[pret-a-porter]

I have created draft PR. I am a bit struggling with unit tests, but you guys can check the general idea.