fix: Throw friendly message for non-object configs #136
Conversation
| */ | ||
| 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]; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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!)
fasttime
left a comment
There was a problem hiding this comment.
LGTM. Since the new changes are causing some tests in ESLint to fail, it would be good to release this in v0.13.x, so we can fix the tests while upgrading the dependency.
Adds explicit validation and clear error messages for:
undefinedconfigsnullconfigsAll of these checks happen during normalization.
refs eslint/eslint#18259