misc(proto): ensure all strings are well-formed #15909
Conversation
| } | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| removeStrings(reportJson); | ||
| iterateStrings(reportJson, (obj, key) => { |
There was a problem hiding this comment.
rest of patch lgtm, but maybe extract this cb into a fn with a name? removeEmptyAndDropLoneSurrogates ?
There was a problem hiding this comment.
Aren't the comments inside sufficient and the code readable? detaching this anonymous fn from the utility function is more jumping around and doesn't help readabilty imo.
There was a problem hiding this comment.
well i suppose this entire file is also in the vein of "read it if you want to know what it does" so.. whatevs
|
|
||
| // Sanitize lone surrogates. | ||
| // @ts-expect-error node 20 | ||
| if (String.prototype.isWellFormed && !value.isWellFormed()) { |
There was a problem hiding this comment.
Could we throw an error if String.prototype.isWellFormed isn't available? Seems like we would want to fail loudly in that case.
There was a problem hiding this comment.
We absolutely do not want to throw an exception here, that would take down PSI for a very minor reason. Anyhow, WRS ships a Chrome that supports this and won't ever regress on that front.
| } | ||
| }); | ||
| } else if (Array.isArray(obj)) { | ||
| obj.forEach(item => { | ||
| if (typeof item === 'object' || Array.isArray(item)) { | ||
| removeStrings(item); | ||
| iterateStrings(item, cb); |
There was a problem hiding this comment.
Technically this allows arrays with non-well-formed strings in them to pass through unchanged.
There was a problem hiding this comment.
ah... same is true for empty strings sticking around. maybe it's fine ... 🪵 🤛
fixes #15908
ref #14911