-
-
Notifications
You must be signed in to change notification settings - Fork 63
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
Add useToJSON
option
#71
Conversation
index.d.ts
Outdated
@@ -28,6 +28,36 @@ export interface Options { | |||
``` | |||
*/ | |||
readonly maxDepth?: number; | |||
|
|||
/** | |||
Indicate whether to use a `toJSON` method if encountered in the object. |
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.
Indicate whether to use a `toJSON` method if encountered in the object. | |
Indicate whether to use a `.toJSON()` method if encountered in the object. |
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 think this should explain in prose when you would want to turn it off.
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.
Done. Also added a link to MDN in the readme https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#tojson_behavior
index.d.ts
Outdated
|
||
toJSON() { | ||
// Break serialization | ||
return {}; |
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.
This does not feel like a realistic example. No one would be doing this.
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.
Do we really need an example? Just explain the scenarios of when it would be useful.
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 dropped the example from the readme too. I suppose this could work in case you change your mind:
```js
import {serializeError} from 'serialize-error';
class CustomError extends Error {
name = 'CustomError';
constructor(message) {
this.time = new Date()
}
toJSON() {
return {
name: 'CustomError'
};
}
}
const error = new CustomError('🦄');
console.log(serializeError(error));
//=> {name: 'CustomError'}
console.log(serializeError(error, {useToJSON: false}));
//=> {name: 'CustomError', message: '🦄', time: '1970-01-01T00:00:00.000Z' stack: 'etc'}
```
amount: `$${this.value}`, | ||
}; | ||
} | ||
} |
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.
} | |
} | |
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.
Too cramped :P
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.
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.
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.
Who reviewed and merged #38? 😜
Can you fix the merge conflict? |
Closes #66