Deprecate value argument in "to have property" #688
Conversation
Arrange for this to contain the logic for showing once. Do a little extra work to make sure the warning is coloured on the console.
lib/utils.js
Outdated
| .text(message, 'bgYellow', 'black') | ||
| .toString(format) | ||
| ); | ||
| }; |
There was a problem hiding this comment.
You can get the output format this way:
const format = expect.outputFormat()So this should also work:
const output = expect.output.clone(expect.outputFormat())There was a problem hiding this comment.
@sunesimonsen that actually doesn't work currently - seems that the method is only allowed on the top-level expect.. should we change that?
There was a problem hiding this comment.
That would be okay with me. Just keep in mind that this method also allow you to give it an argument to change the default format.
lib/utils.js
Outdated
| return; | ||
| } | ||
|
|
||
| deprecationWarningDisplayed = true; |
There was a problem hiding this comment.
I don't think we should be cleaver about how much we show that warning.
There was a problem hiding this comment.
Then we can make a proper style for the warning in styles.js and make a util for printing them to the console.
There was a problem hiding this comment.
@sunesimonsen cool. I only added the "once only" based on what I saw -sinon do (https://github.com/unexpectedjs/unexpected-sinon/blob/c1bb48c459d6c8d501d8a63ca4c33a059aea6965/lib/unexpected-sinon.js#L1088) :)
Create a deprecationWarning style used for the yellow on black colouring. Keep things simple (per review) and remove the one-time only check which further reduces things down to a simple call. Pass the expect instance for its .output rather than requiring magicpen.
This commit adds a small utility function that can be used when we add deprecation notices for assertions. I've also added a nice deprecation notice for that value argument in "to have property", outputted once, that informs the user what to do:
