feat: dynamic styles set inherits to false

[Samantha-Zhan]

What changed / motivation ?

Addresses task #734.

Generate @property declarations for all variables that are created for dynamic styles and declare them as not inheriting. Increase stylex variable compiler efficiency by disabling all dynamic styles from cascading down from the node defining the variable.

Linked PR/Issues

Fixes #734

Additional Context

npx jest stylex-transform-create-test

Test file stylex-transform-create-test.js should have @property injection added for all stylex dynamic styles.

Pre-flight checklist

• I have read the contributing guidelines
  Contribution Guidelines
• Performed a self-review of my code

[github-actions]

workflow: benchmarks/size

Comparison of minified (terser) and compressed (brotli) size results, measured in bytes. Smaller is better.

@stylexjs/[email protected] size:compare
./size-compare.js /tmp/tmp.BcB1G81fFf /tmp/tmp.2zUBwnUong

Results	Base	Patch	Ratio
stylex/lib/stylex.js
· compressed	729	729	1.00
· minified	2,541	2,541	1.00
stylex/lib/StyleXSheet.js
· compressed	1,266	1,266	1.00
· minified	3,776	3,776	1.00
rollup-example/.build/bundle.js
· compressed	562,983	562,983	1.00
· minified	10,185,368	10,185,368	1.00
rollup-example/.build/stylex.css
· compressed	99,229	99,229	1.00
· minified	745,845	745,845	1.00

[nmn]

Make sure you add this code after the if condition that checks confident. Otherwise you'll end up throwing errors because fns will be null or undefined.

Suggested change

	// add injection that mark variables used for dynamic styles as `inherits: false`
	const injectedInheritStyles: { [string]: InjectableStyle } = {};
	if (fns != null) {
	const dynamicFnsNames = Object.values(fns)
	?.map((entry) => Object.keys(entry[1]))
	.flat();
	dynamicFnsNames.forEach((fnsName) => {
	injectedInheritStyles[fnsName] = {
	priority: 0,
	ltr: `@property ${fnsName} { inherits: false }`,
	rtl: null,
	};
	});
	}
	if (!confident) {
	throw path.buildCodeFrameError(messages.NON_STATIC_VALUE, SyntaxError);
	}
	if (!confident) {
	throw path.buildCodeFrameError(messages.NON_STATIC_VALUE, SyntaxError);
	}
	// add injection that mark variables used for dynamic styles as `inherits: false`
	const injectedInheritStyles: { [string]: InjectableStyle } = {};
	if (fns != null) {
	const dynamicFnsNames = Object.values(fns)
	?.map((entry) => Object.keys(entry[1]))
	.flat();
	dynamicFnsNames.forEach((fnsName) => {
	injectedInheritStyles[fnsName] = {
	priority: 0,
	ltr: `@property ${fnsName} { inherits: false }`,
	rtl: null,
	};
	});
	}

[Samantha-Zhan]

Great call, thanks for fixing it!

[necolas]

I noticed that this change didn't show up in the CSS bundle size benchmark. We don't have any dynamic styles syntax used in the fixture data. It would be good to capture the impact on bundle size by updating the benchmark's fixture data to include a significant number of dynamic styles. (The original fixture was scraped from a snapshot of Comet that pre-dates support for dynamic styles.)

When using inline styles directly, you don't incur the runtime cost of using custom properties that necessitates this patch. For most dynamic use cases, we can simply render inline styles without using custom properties or adding generated CSS. There are runtime and bundle size wins to be had there. With improved fixture data we should be able to get a sense of how to quantify the impact of future optimizations to the implementation.

[necolas]

Created new tasks: #799, #800