fix(serializer): normalize style attributes

[jmsjtu]

Split into two commits to make the changes more obvious.

See #236 for class changes.

BREAKING CHANGE: snapshots may change.

W-15365974

[nolanlawson]

We should probably split on ; and normalize whitespace for each, since there can be arbitrary whitespace before/after the semicolon.

[nolanlawson]

Also this would be cleaner to go in its own cleanElementStyle function/file imo.

[nolanlawson]

Also: this is not going to handle zany things like content or grid-template-areas which may contain spaces or semicolons in the value, but I'm not sure that that's worth handling. It quickly devolves into prettier/full CSS parsing, and we probably just need a lightweight formatter.

[jmsjtu]

@nolanlawson I've updated the logic so reduce the spaces when there is a ';'.

I tried to preserve the space if it's preceded by a ';'

So something like this:

<div style="color: blue;   text-align: center"></div>

will output as:

<div style="color: blue; text-align: center"></div>

See the tests for different variations, let me know what you think, alternatively we can just strip all the spaces before/after a ';'.

Also added some logic to replace /t and /n.

[nolanlawson]

Suggested change

	.split(';')
	// Preserve space character following a ';', style="color: blue; text-align: center"
	.map((attr) => attr.trimEnd())
	.join(';');
	.split(';')
	.map(_ => _.trim())
	.filter(Boolean)
	.map(_ => `${_};`)
	.join(' ');

This avoids the duplicates ;s at the end.

[wjhsf]

Suggested change

	.split(';')
	// Preserve space character following a ';', style="color: blue; text-align: center"
	.map((attr) => attr.trimEnd())
	.join(';');
	.split(';')
	.map(_ => `${_.trim()};`)
	.filter(_ => _ !== ';')
	.join(' ');

Same but one less loop.

[jmsjtu]

This will also append a ';' at the end of a style:

<div style="  color: blue  "></div>

Becomes:

<div style="color: blue;"></div>

Empty ';' will be removed as well

<div style=";;"></div>

Becomes:

<div />

Just want to double-check, are we trying to normalize the whitespace or completely format the style attribute output?

Asking because it won't exactly line up with what the component author will see in the browser.

Actually, on second thought, I don't think ☝️ will be an issue

[nolanlawson]

Yes, I think we want consistent output. So consistent ;s at the end, consistent spacing, and just <div /> if the style is semantically empty.

[nolanlawson]

text-align : center

This one is going to lead to inconsistencies between static/dynamic sadly... Not sure how to solve without a full CSS parse 🤦

[nolanlawson]

On second thought, we can't use a CSS parser because we've never validated this content, so it could be syntactically invalid and consumers could be relying on that.

I think we should split on : as well and normalize that so that it's always in the format:

color: red;

or

color: red !important;

[nolanlawson]

#238

[nolanlawson]

This doesn't work well for cases like this:

<div style="background-image: url(data: image/svg+xml;base64,abc123); background-size: 12px;"></div>

This incorrectly handles the ;s, serializing into:

<div
     style="background-image: url(data: image/svg+xml; base64,abc123); background-size: 12px;"
/>

[nolanlawson]

See above and #239