Skip to content

beforeMigration option #167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 30, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,46 @@ const store = new Conf({

> Note: The version the migrations use refers to the **project version** by default. If you want to change this behavior, specify the [`projectVersion`](#projectVersion) option.

### beforeMigration

Type: `Function`\
Default: `undefined`

You can use `beforeMigration` callback for logging purposes, preparing migration data, etc.

`beforeMigration` callback will be called before the migration is executed.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ambiguous. Before the whole process or before each step.


It could be useful when u have multiple configs and you want to log the migration data for each config.

Example:

```js
const Conf = require('conf');

console.log = someLogger.log;

const mainConfig = new Conf({
beforeMigration: (store, currentVersion, nextVersion) => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if it were like this.

Suggested change
beforeMigration: (store, currentVersion, nextVersion) => {
beforeMigration: (store, context) => {

That way we could easily add more metadata properties in the future.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the properties maybe be named fromVersion and toVersion? currentVersion makes it sound like it's the current version of the package, not the current version in the migration process.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could maybe also add the package version and all the migration versions. In case the user wants to know where in the migration process they are.

Copy link
Contributor Author

@Ciberusps Ciberusps Jul 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could maybe also add the package version and all the migration versions. In case the user wants to know where in the migration process they are.

Added context.migrationProcess with finalVersion and migrations not sure how to name them properly probably they can be flattened and lay in context but proper naming required

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to keep them top-level in context (no nesting).

console.log(`[main-config] migrate from ${currentVersion} -> ${nextVersion}`);
},
migrations: {
'0.4.0': store => {
store.set('debugPhase', true);
},
}
});
const secondConfig = new Conf({
beforeMigration: (store, currentVersion, nextVersion) => {
console.log(`[second-config] migrate from ${currentVersion} -> ${nextVersion}`);
},
migrations: {
'1.0.1': store => {
store.set('debugPhase', true);
},
}
});
```

#### configName

Type: `string`\
Expand Down
14 changes: 9 additions & 5 deletions source/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ import debounceFn = require('debounce-fn');
import semver = require('semver');
import onetime = require('onetime');
import {JSONSchema} from 'json-schema-typed';
import {Deserialize, Migrations, OnDidChangeCallback, Options, Serialize, Unsubscribe, Schema, OnDidAnyChangeCallback} from './types';
import {Deserialize, Migrations, OnDidChangeCallback, Options, Serialize, Unsubscribe, Schema, OnDidAnyChangeCallback, BeforeMigrationCallback} from './types';

const encryptionAlgorithm = 'aes-256-cbc';

const createPlainObject = <T = unknown>(): T => {
const createPlainObject = <T = Record<string, unknown>>(): T => {
return Object.create(null);
};

Expand Down Expand Up @@ -141,7 +141,7 @@ class Conf<T extends Record<string, any> = Record<string, unknown>> implements I
this.path = path.resolve(options.cwd, `${options.configName ?? 'config'}${fileExtension}`);

const fileStore = this.store;
const store = Object.assign(createPlainObject<T>(), options.defaults, fileStore);
const store = Object.assign(createPlainObject(), options.defaults, fileStore);
this._validate(store);

try {
Expand All @@ -163,7 +163,7 @@ class Conf<T extends Record<string, any> = Record<string, unknown>> implements I
throw new Error('Project version could not be inferred. Please specify the `projectVersion` option.');
}

this._migrate(options.migrations, options.projectVersion);
this._migrate(options.migrations, options.projectVersion, options.beforeMigration);
}
}

Expand Down Expand Up @@ -499,7 +499,7 @@ class Conf<T extends Record<string, any> = Record<string, unknown>> implements I
}
}

private _migrate(migrations: Migrations<T>, versionToMigrate: string): void {
private _migrate(migrations: Migrations<T>, versionToMigrate: string, beforeMigration?: BeforeMigrationCallback<T>): void {
let previousMigratedVersion = this._get(MIGRATION_KEY, '0.0.0');

const newerVersions = Object.keys(migrations)
Expand All @@ -509,6 +509,10 @@ class Conf<T extends Record<string, any> = Record<string, unknown>> implements I

for (const version of newerVersions) {
try {
if (beforeMigration) {
beforeMigration(this, previousMigratedVersion, version);
}

const migration = migrations[version];
migration(this);

Expand Down
11 changes: 11 additions & 0 deletions source/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,16 @@ export interface Options<T extends Record<string, any>> {
*/
migrations?: Migrations<T>;

/**
You can use `beforeMigration` callback for logging purposes, preparing migration data, etc.

`beforeMigration` callback will be called before the migration is executed.

It could be useful when u have multiple configs and you want to log the migration data for each config.

*/
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
*/
*/

beforeMigration?: BeforeMigrationCallback<T>;

/**
__You most likely don't need this. Please don't use it unless you really have to.__

Expand Down Expand Up @@ -226,6 +236,7 @@ export interface Options<T extends Record<string, any>> {
}

export type Migrations<T extends Record<string, any>> = Record<string, (store: Conf<T>) => void>;
export type BeforeMigrationCallback<T extends Record<string, any>> = (store: Conf<T>, currentVersion: string, previousVersion: string) => void;

export type Schema<T> = {[Property in keyof T]: ValueSchema};
export type ValueSchema = TypedJSONSchema;
Expand Down
4 changes: 4 additions & 0 deletions test/index.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,10 @@ expectError<number>(config.get('unicorn', 1));

// -- Migrations --
new Conf({
beforeMigration: (store, currentVersion, nextVersion) => {
console.log(`[main-config] migrate from ${currentVersion} -> ${nextVersion}`);
console.log(`[main-config] phase ${(store.get('phase') || 'none') as string}`);
},
migrations: {
'0.0.1': store => {
store.set('debug phase', true);
Expand Down
19 changes: 19 additions & 0 deletions test/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1203,3 +1203,22 @@ test('__internal__ keys - should only match specific "__internal__" entry', t =>
conf.set('__internal__foo.you-shall', 'not-pass');
});
});

test('beforeMigration - should be called before every migration', t => {
const conf = new Conf({
cwd: tempy.directory(),
projectVersion: '2.0.0',
beforeMigration: (store, currentVersion, nextVersion) => {
store.set(`beforeMigration ${currentVersion} -> ${nextVersion}`, true);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
store.set(`beforeMigration ${currentVersion} -> ${nextVersion}`, true);
store.set(`beforeMigration ${currentVersion} ${nextVersion}`, true);

},
migrations: {
'1.0.0': () => {},
'1.0.1': () => {},
'2.0.1': () => {}
}
});

t.is(conf.get('beforeMigration 0.0.0 -> 1.0.0'), true);
t.is(conf.get('beforeMigration 1.0.0 -> 1.0.1'), true);
t.false(conf.has('beforeMigration 1.0.1 -> 2.0.1'));
});