fix(utils): Normalize undefined to undefined instead of "[undefined]"
#8017
Conversation
lforst
changed the title
undefined to undefined instead of "undefined"undefined to undefined instead of "[undefined]"
size-limit report 📦
|
There was a problem hiding this comment.
I was wondering before why we even had that in the first place - IMHO usually you'll not care to see this, as it should be identical to not having this, and when JSON stringifying undefined things are normally dropped. You'll have some more tests to fix, though 😅
packages/utils/src/normalize.ts
Outdated
| value === undefined || | ||
| value === null || |
There was a problem hiding this comment.
super-l (just with bundle size in mind but feel free to ignore):
| value === undefined || | |
| value === null || | |
| value == null || |
This should be identical, just a little shorter, right?
There was a problem hiding this comment.
You're right. I even thought about this but I stopped micro-optimizing for bundle because I assume terser will optimize this stuff away anyhow. Here I am still gonna go with your change because I don't know if it catches ths.
| @@ -366,28 +366,26 @@ describe('normalize()', () => { | |||
| }); | |||
|
|
|||
| test('primitive values in objects/arrays', () => { | |||
| expect(normalize(['foo', 42, undefined, NaN])).toEqual(['foo', 42, '[undefined]', '[NaN]']); | |||
| expect(normalize(['foo', 42, NaN])).toEqual(['foo', 42, '[NaN]']); | |||
There was a problem hiding this comment.
I'm just thinking why not keep the undefined here?
| expect(normalize(['foo', 42, NaN])).toEqual(['foo', 42, '[NaN]']); | |
| expect(normalize(['foo', 42, undefined, NaN])).toEqual(['foo', 42, undefined, '[NaN]']); |
I would expect this to pass, or not?
Fixes #7933
Supersedes #8009
It's a bit weird that undefined normalizes to a string and even causes issues for users (see #7933). I suggest we change this.