Skip to content
This repository has been archived by the owner on Jun 10, 2024. It is now read-only.

fix: Throw friendly message for non-object configs #136

Merged
merged 2 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
48 changes: 31 additions & 17 deletions src/config-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,19 @@ class ConfigError extends Error {
* @param {number} index The index of the config object in the array.
* @param {Error} source The source error.
*/
constructor(name, index, source) {
super(`Config ${name}: ${source.message}`, { cause: source });
constructor(name, index, { cause, message }) {


const finalMessage = message || cause.message;

super(`Config ${name}: ${finalMessage}`, { cause });

// copy over custom properties that aren't represented
for (const key of Object.keys(source)) {
if (!(key in this)) {
this[key] = source[key];
if (cause) {
for (const key of Object.keys(cause)) {
if (!(key in this)) {
this[key] = cause[key];
}
}
}
Comment on lines -54 to 67
Copy link
Contributor

Choose a reason for hiding this comment

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

Previously, when an Error object was passed as source, this code copied the properties from that Error object and set that Error object as the cause of the ConfigError. Now it's copying the properties from Error.cause and setting Error.cause as the cause of the ConfigError, so the properties of the Error object are lost and the Error object won't be in the cause chain. I'm not yet sure if this is causing some observable differences in functionalities at the moment, but it might be safer to revert to the previous behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the function signature but forgot to update the call sites to use the new signature. It should work the same now. (I guess this is where type checking would have been helpful!)


Expand All @@ -82,14 +88,13 @@ class ConfigError extends Error {
* @returns {string} The name of the config object.
*/
function getConfigName(config) {
if (typeof config.name === 'string' && config.name) {
if (config && typeof config.name === 'string' && config.name) {
return `"${config.name}"`;
}

return '(unnamed)';
}


/**
* Rethrows a config error with additional information about the config object.
* @param {object} config The config object to get the name of.
Expand All @@ -112,19 +117,28 @@ function isString(value) {
}

/**
* Creates a function that asserts that the files and ignores keys
* of a config object are valid as per base schema.
* Creates a function that asserts that the config is valid
* during normalization. This checks that the config is not nullish
* and that files and ignores keys of a config object are valid as per base schema.
* @param {Object} config The config object to check.
* @param {number} index The index of the config object in the array.
* @returns {void}
* @throws {ConfigError} If the files and ignores keys of a config object are not valid.
*/
function assertValidFilesAndIgnores(config, index) {
function assertValidBaseConfig(config, index) {

if (!config || typeof config !== 'object') {
return;
if (config === null) {
throw new ConfigError(getConfigName(config), index, { message: 'Unexpected null config.' });
}


if (config === undefined) {
throw new ConfigError(getConfigName(config), index, { message: 'Unexpected undefined config.' });
}

if (typeof config !== 'object') {
throw new ConfigError(getConfigName(config), index, { message: 'Unexpected non-object config.' });
}

const validateConfig = { };

if ('files' in config) {
Expand All @@ -138,7 +152,7 @@ function assertValidFilesAndIgnores(config, index) {
try {
FILES_AND_IGNORES_SCHEMA.validate(validateConfig);
} catch (validationError) {
rethrowConfigError(config, index, validationError);
rethrowConfigError(config, index, { cause: validationError });
}
}

Expand Down Expand Up @@ -634,7 +648,7 @@ export class ConfigArray extends Array {
const normalizedConfigs = await normalize(this, context, this.extraConfigTypes);
this.length = 0;
this.push(...normalizedConfigs.map(this[ConfigArraySymbol.preprocessConfig].bind(this)));
this.forEach(assertValidFilesAndIgnores);
this.forEach(assertValidBaseConfig);
this[ConfigArraySymbol.isNormalized] = true;

// prevent further changes
Expand All @@ -656,7 +670,7 @@ export class ConfigArray extends Array {
const normalizedConfigs = normalizeSync(this, context, this.extraConfigTypes);
this.length = 0;
this.push(...normalizedConfigs.map(this[ConfigArraySymbol.preprocessConfig].bind(this)));
this.forEach(assertValidFilesAndIgnores);
this.forEach(assertValidBaseConfig);
this[ConfigArraySymbol.isNormalized] = true;

// prevent further changes
Expand Down Expand Up @@ -892,7 +906,7 @@ export class ConfigArray extends Array {
try {
return this[ConfigArraySymbol.schema].merge(result, this[index]);
} catch (validationError) {
rethrowConfigError(this[index], index, validationError);
rethrowConfigError(this[index], index, { cause: validationError});
}
}, {}, this);

Expand Down
53 changes: 50 additions & 3 deletions tests/config-array.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -416,13 +416,12 @@ describe('ConfigArray', () => {
},
'eslint:reccommended' // typo
], { basePath });
await configs.normalize();

expect(() => {
configs.getConfig(path.resolve(basePath, 'foo.js'));
configs.normalizeSync();
})
.to
.throw('All arguments must be objects.');
.throw('Config (unnamed): Unexpected non-object config.');

});

Expand All @@ -433,6 +432,7 @@ describe('ConfigArray', () => {
name: true
}
], { basePath });

await configs.normalize();

expect(() => {
Expand All @@ -451,6 +451,7 @@ describe('ConfigArray', () => {
name: true
}
);

await configs.normalize();

expect(() => {
Expand All @@ -460,6 +461,52 @@ describe('ConfigArray', () => {
.throw('Config (unnamed): Key "name": Property must be a string.');

});

it('should throw an error when base config is undefined', async () => {
configs = new ConfigArray([undefined], { basePath });

expect(() => {
configs.normalizeSync();
})
.to
.throw('Config (unnamed): Unexpected undefined config.');

});

it('should throw an error when base config is null', async () => {
configs = new ConfigArray([null], { basePath });

expect(() => {
configs.normalizeSync();
})
.to
.throw('Config (unnamed): Unexpected null config.');

});

it('should throw an error when additional config is undefined', async () => {
configs = new ConfigArray([{}], { basePath });
configs.push(undefined);

expect(() => {
configs.normalizeSync();
})
.to
.throw('Config (unnamed): Unexpected undefined config.');

});

it('should throw an error when additional config is null', async () => {
configs = new ConfigArray([{}], { basePath });
configs.push(null);

expect(() => {
configs.normalizeSync();
})
.to
.throw('Config (unnamed): Unexpected null config.');

});
});

describe('ConfigArray members', () => {
Expand Down
Loading