-
-
Notifications
You must be signed in to change notification settings - Fork 12
fix: Throw friendly message for non-object configs #136
Conversation
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.
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?
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.
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!)
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.
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.
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.
LGTM, thanks!
Adds explicit validation and clear error messages for:
undefined
configsnull
configsAll of these checks happen during normalization.
refs eslint/eslint#18259