qunitjs / js-reporters

📋 Common Reporter Interface (CRI) for JavaScript testing frameworks.
MIT License
59 stars 18 forks source link

Use null instead of undefined for JSON compat #124

Closed Krinkle closed 3 years ago

Krinkle commented 3 years ago

While writing adapters and tests, I find myself running into confusing diffs and return values due to objects like { foo: 1, bar: undefined } turning into { foo: 1 } after standard JSON serialisation, because undefined is not valid in JSON.

I'm proposing to change our use of undefined in the spec to null instead.

Specifically:

There is one more mention of undefined in event data, namely for Assertion.stack.

  • string|undefined stack: Optional stack trace. For a "passed" assertion, it is always undefined.

Unlike for global suites, where undefined is somewhat meaningful, I think for assertions the undefined here was meant to allow implementers to omit the property, although it doesn't say that currently. From a lazy perpective though, absence and setting to undefined are mostly the same thing. Requiring setting of null as fallback would move further away from optional properties.

For this one, I'm proposing to introduce our first truely optional event data property and specify Assertion.stack as optional property of type string. In practice, setting undefined or null will likely be compatible with most reporters, but we wouldn't recommend that.