From bca06e9b239a544e512f5a418b8f56153d6e5ca0 Mon Sep 17 00:00:00 2001 From: Naman Goel Date: Tue, 23 Jul 2024 03:58:25 +0530 Subject: [PATCH 1/4] feat: Allow "var(--" strings as values for all properties --- packages/dev-runtime/src/stylex-create.js | 1 - .../eslint-plugin/src/stylex-valid-shorthands.js | 1 + packages/shared/src/index.js | 15 +++++++++------ packages/shared/src/physical-rtl/generate-ltr.js | 10 ++++++---- packages/shared/src/physical-rtl/generate-rtl.js | 4 ++-- .../src/preprocess-rules/basic-validation.js | 3 +-- packages/shared/src/preprocess-rules/index.js | 1 + packages/shared/src/transform-value.js | 2 +- packages/shared/src/utils/property-priorities.js | 1 + packages/stylex/src/StyleXCSSTypes.js | 2 +- 10 files changed, 23 insertions(+), 17 deletions(-) diff --git a/packages/dev-runtime/src/stylex-create.js b/packages/dev-runtime/src/stylex-create.js index f4c3268f..a8fa37a3 100644 --- a/packages/dev-runtime/src/stylex-create.js +++ b/packages/dev-runtime/src/stylex-create.js @@ -124,7 +124,6 @@ function createWithFns( if (origKey.startsWith('var(') && origKey.endsWith(')')) { key = origKey.slice(4, -1); } - if (typeof value === 'function') { const [staticObj, inlineStylesFn] = splitStaticObj( value as $FlowFixMe, diff --git a/packages/eslint-plugin/src/stylex-valid-shorthands.js b/packages/eslint-plugin/src/stylex-valid-shorthands.js index 07dfec39..1134f18a 100644 --- a/packages/eslint-plugin/src/stylex-valid-shorthands.js +++ b/packages/eslint-plugin/src/stylex-valid-shorthands.js @@ -129,6 +129,7 @@ const stylexValidShorthands = { if (typeof key === 'string' && legacyNameMapping[key] != null) { context.report({ node: property, + // $FlowFixMe[invalid-computed-prop] message: `Use "${legacyNameMapping[key]}" instead of legacy formats like "${key}" to adhere to logical property naming.`, fix: (fixer) => { // $FlowFixMe - We've already checked that key is a string and in legacyNameMapping diff --git a/packages/shared/src/index.js b/packages/shared/src/index.js index e634ee5f..3957d0fb 100644 --- a/packages/shared/src/index.js +++ b/packages/shared/src/index.js @@ -60,12 +60,15 @@ export const utils: { export const messages: typeof m = m; export const IncludedStyles: typeof _IncludedStyles = _IncludedStyles; export const firstThatWorks: typeof stylexFirstThatWorks = stylexFirstThatWorks; -export const PSEUDO_CLASS_PRIORITIES: typeof _PSEUDO_CLASS_PRIORITIES = - _PSEUDO_CLASS_PRIORITIES; -export const AT_RULE_PRIORITIES: typeof _AT_RULE_PRIORITIES = - _AT_RULE_PRIORITIES; -export const PSEUDO_ELEMENT_PRIORITY: typeof _PSEUDO_ELEMENT_PRIORITY = - _PSEUDO_ELEMENT_PRIORITY; +export const PSEUDO_CLASS_PRIORITIES: $ReadOnly<{ + +[string]: ?number, + ...typeof _PSEUDO_CLASS_PRIORITIES, +}> = _PSEUDO_CLASS_PRIORITIES; +export const AT_RULE_PRIORITIES: $ReadOnly<{ + +[string]: ?number, + ...typeof _AT_RULE_PRIORITIES, +}> = _AT_RULE_PRIORITIES; +export const PSEUDO_ELEMENT_PRIORITY: number = _PSEUDO_ELEMENT_PRIORITY; export type InjectableStyle = _InjectableStyle; export type CompiledNamespaces = _CompiledNamespaces; diff --git a/packages/shared/src/physical-rtl/generate-ltr.js b/packages/shared/src/physical-rtl/generate-ltr.js index bcf15401..2514390c 100644 --- a/packages/shared/src/physical-rtl/generate-ltr.js +++ b/packages/shared/src/physical-rtl/generate-ltr.js @@ -7,13 +7,13 @@ * @flow strict */ -const logicalToPhysical: $ReadOnly<{ [string]: string }> = { +const logicalToPhysical: $ReadOnly<{ [string]: ?string }> = { start: 'left', end: 'right', }; const propertyToLTR: $ReadOnly<{ - [key: string]: ($ReadOnly<[string, string]>) => $ReadOnly<[string, string]>, + [key: string]: ?($ReadOnly<[string, string]>) => $ReadOnly<[string, string]>, }> = { 'margin-start': ([_key, val]: $ReadOnly<[string, string]>) => [ 'margin-left', @@ -122,8 +122,10 @@ export default function generateLTR( pair: $ReadOnly<[string, string]>, ): $ReadOnly<[string, string]> { const [key] = pair; - if (propertyToLTR[key]) { - return propertyToLTR[key](pair); + + const property = propertyToLTR[key]; + if (property) { + return property(pair); } return pair; } diff --git a/packages/shared/src/physical-rtl/generate-rtl.js b/packages/shared/src/physical-rtl/generate-rtl.js index 75aff072..b886bb05 100644 --- a/packages/shared/src/physical-rtl/generate-rtl.js +++ b/packages/shared/src/physical-rtl/generate-rtl.js @@ -9,7 +9,7 @@ import parser from 'postcss-value-parser'; -const cursorFlip: $ReadOnly<{ [string]: string }> = { +const cursorFlip: $ReadOnly<{ [string]: ?string }> = { 'e-resize': 'w-resize', 'w-resize': 'e-resize', 'ne-resize': 'nw-resize', @@ -98,7 +98,7 @@ const logicalToPhysical: $ReadOnly<{ [string]: ?string }> = { }; const propertyToRTL: $ReadOnly<{ - [key: string]: ( + [key: string]: ?( $ReadOnly<[string, string]>, ) => $ReadOnly<[string, string]> | null, }> = { diff --git a/packages/shared/src/preprocess-rules/basic-validation.js b/packages/shared/src/preprocess-rules/basic-validation.js index c0351bd8..de9b9ba3 100644 --- a/packages/shared/src/preprocess-rules/basic-validation.js +++ b/packages/shared/src/preprocess-rules/basic-validation.js @@ -63,8 +63,7 @@ function validateConditionalStyles( val: ConditionalStyles, conditions: $ReadOnlyArray = [], ): void { - for (const key in val) { - const v = val[key]; + for (const [key, v] of Object.entries(val)) { if (!(key.startsWith('@') || key.startsWith(':') || key === 'default')) { throw new Error(messages.INVALID_PSEUDO_OR_AT_RULE); } diff --git a/packages/shared/src/preprocess-rules/index.js b/packages/shared/src/preprocess-rules/index.js index cd91d632..060f7d6d 100644 --- a/packages/shared/src/preprocess-rules/index.js +++ b/packages/shared/src/preprocess-rules/index.js @@ -39,6 +39,7 @@ export default function flatMapExpandedShorthands( const expansion: ( string | number | null, ) => $ReadOnlyArray<[string, TStyleValue]> = + // $FlowFixMe[invalid-computed-prop] expansions[options.styleResolution ?? 'application-order'][key]; if (expansion) { if (Array.isArray(value)) { diff --git a/packages/shared/src/transform-value.js b/packages/shared/src/transform-value.js index c22e104b..35d62e60 100644 --- a/packages/shared/src/transform-value.js +++ b/packages/shared/src/transform-value.js @@ -115,7 +115,7 @@ const unitlessNumberProperties = new Set([ ]); // List of properties that have custom suffixes for numbers -const numberPropertySuffixes: { +[key: string]: string } = { +const numberPropertySuffixes: { +[key: string]: ?string } = { animationDelay: 'ms', animationDuration: 'ms', transitionDelay: 'ms', diff --git a/packages/shared/src/utils/property-priorities.js b/packages/shared/src/utils/property-priorities.js index 639f1c2a..c780ee8d 100644 --- a/packages/shared/src/utils/property-priorities.js +++ b/packages/shared/src/utils/property-priorities.js @@ -749,6 +749,7 @@ export default function getPriority(key: string): number { ? key.slice(0, key.indexOf('(')) : key; + // $FlowFixMe[invalid-computed-prop] return PSEUDO_CLASS_PRIORITIES[prop] ?? 40; } diff --git a/packages/stylex/src/StyleXCSSTypes.js b/packages/stylex/src/StyleXCSSTypes.js index 60909039..9527a630 100644 --- a/packages/stylex/src/StyleXCSSTypes.js +++ b/packages/stylex/src/StyleXCSSTypes.js @@ -100,7 +100,7 @@ type alignSelf = | 'safe center' | 'unsafe center' | all; -type all = null | 'initial' | 'inherit' | 'unset'; +type all = null | 'initial' | 'inherit' | 'unset' | StringPrefix<'var(--'>; type animationDelay = time; type animationDirection = singleAnimationDirection; type animationDuration = time; From 29561006551373f03e20f86074588c4ec2f31f5e Mon Sep 17 00:00:00 2001 From: Naman Goel Date: Tue, 23 Jul 2024 21:01:03 +0530 Subject: [PATCH 2/4] fix: type error --- packages/shared/src/stylex-define-vars.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/shared/src/stylex-define-vars.js b/packages/shared/src/stylex-define-vars.js index a6463354..3bde1924 100644 --- a/packages/shared/src/stylex-define-vars.js +++ b/packages/shared/src/stylex-define-vars.js @@ -74,6 +74,7 @@ export default function styleXDefineVars( ); const injectableStyles = constructCssVariablesString( + // $FlowFixMe[incompatible-call] - Can't use a class when a plain object is expected variablesMap, themeNameHash, ); From 3f46435fe551f020423a26ae63207384bace15fd Mon Sep 17 00:00:00 2001 From: Naman Goel Date: Tue, 23 Jul 2024 21:09:40 +0530 Subject: [PATCH 3/4] fix: lint error --- packages/rollup-plugin/flow_modules/rollup/index.js | 2 +- packages/shared/src/physical-rtl/generate-rtl.js | 5 +++-- packages/shared/src/utils/property-priorities.js | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/rollup-plugin/flow_modules/rollup/index.js b/packages/rollup-plugin/flow_modules/rollup/index.js index 0473f977..29239a9c 100644 --- a/packages/rollup-plugin/flow_modules/rollup/index.js +++ b/packages/rollup-plugin/flow_modules/rollup/index.js @@ -525,7 +525,7 @@ export type SequentialPluginHooks = | 'transform'; export type ParallelPluginHooks = Exclude< - $Keys | $Keys, + $Keys | AddonHooks, FirstPluginHooks | SequentialPluginHooks, >; diff --git a/packages/shared/src/physical-rtl/generate-rtl.js b/packages/shared/src/physical-rtl/generate-rtl.js index b886bb05..13b674f8 100644 --- a/packages/shared/src/physical-rtl/generate-rtl.js +++ b/packages/shared/src/physical-rtl/generate-rtl.js @@ -181,8 +181,9 @@ const propertyToRTL: $ReadOnly<{ export default function generateRTL([key, value]: $ReadOnly< [string, string], >): ?$ReadOnly<[string, string]> { - if (propertyToRTL[key]) { - return propertyToRTL[key]([key, value]); + const toRTLForKey = propertyToRTL[key]; + if (toRTLForKey) { + return toRTLForKey([key, value]); } return null; } diff --git a/packages/shared/src/utils/property-priorities.js b/packages/shared/src/utils/property-priorities.js index c780ee8d..3ffb3e96 100644 --- a/packages/shared/src/utils/property-priorities.js +++ b/packages/shared/src/utils/property-priorities.js @@ -645,7 +645,7 @@ longHandLogical.add('touch-action'); // ':active': 170, // }; -export const PSEUDO_CLASS_PRIORITIES: $ReadOnly<{ [string]: number }> = { +export const PSEUDO_CLASS_PRIORITIES: $ReadOnly<{ [string]: ?number }> = { ':is': 40, ':where': 40, ':not': 40, From 2c83553f228fe5ea8d4f5e073eeb07b5446e5a09 Mon Sep 17 00:00:00 2001 From: Naman Goel Date: Tue, 10 Sep 2024 01:20:56 -0700 Subject: [PATCH 4/4] fix: remove unneeded changes --- packages/dev-runtime/src/stylex-create.js | 1 + .../eslint-plugin/src/stylex-valid-shorthands.js | 1 - packages/shared/src/index.js | 15 ++++++--------- packages/shared/src/preprocess-rules/index.js | 1 - packages/shared/src/stylex-define-vars.js | 1 - packages/shared/src/utils/property-priorities.js | 1 - 6 files changed, 7 insertions(+), 13 deletions(-) diff --git a/packages/dev-runtime/src/stylex-create.js b/packages/dev-runtime/src/stylex-create.js index a8fa37a3..f4c3268f 100644 --- a/packages/dev-runtime/src/stylex-create.js +++ b/packages/dev-runtime/src/stylex-create.js @@ -124,6 +124,7 @@ function createWithFns( if (origKey.startsWith('var(') && origKey.endsWith(')')) { key = origKey.slice(4, -1); } + if (typeof value === 'function') { const [staticObj, inlineStylesFn] = splitStaticObj( value as $FlowFixMe, diff --git a/packages/eslint-plugin/src/stylex-valid-shorthands.js b/packages/eslint-plugin/src/stylex-valid-shorthands.js index 1134f18a..07dfec39 100644 --- a/packages/eslint-plugin/src/stylex-valid-shorthands.js +++ b/packages/eslint-plugin/src/stylex-valid-shorthands.js @@ -129,7 +129,6 @@ const stylexValidShorthands = { if (typeof key === 'string' && legacyNameMapping[key] != null) { context.report({ node: property, - // $FlowFixMe[invalid-computed-prop] message: `Use "${legacyNameMapping[key]}" instead of legacy formats like "${key}" to adhere to logical property naming.`, fix: (fixer) => { // $FlowFixMe - We've already checked that key is a string and in legacyNameMapping diff --git a/packages/shared/src/index.js b/packages/shared/src/index.js index 3957d0fb..e634ee5f 100644 --- a/packages/shared/src/index.js +++ b/packages/shared/src/index.js @@ -60,15 +60,12 @@ export const utils: { export const messages: typeof m = m; export const IncludedStyles: typeof _IncludedStyles = _IncludedStyles; export const firstThatWorks: typeof stylexFirstThatWorks = stylexFirstThatWorks; -export const PSEUDO_CLASS_PRIORITIES: $ReadOnly<{ - +[string]: ?number, - ...typeof _PSEUDO_CLASS_PRIORITIES, -}> = _PSEUDO_CLASS_PRIORITIES; -export const AT_RULE_PRIORITIES: $ReadOnly<{ - +[string]: ?number, - ...typeof _AT_RULE_PRIORITIES, -}> = _AT_RULE_PRIORITIES; -export const PSEUDO_ELEMENT_PRIORITY: number = _PSEUDO_ELEMENT_PRIORITY; +export const PSEUDO_CLASS_PRIORITIES: typeof _PSEUDO_CLASS_PRIORITIES = + _PSEUDO_CLASS_PRIORITIES; +export const AT_RULE_PRIORITIES: typeof _AT_RULE_PRIORITIES = + _AT_RULE_PRIORITIES; +export const PSEUDO_ELEMENT_PRIORITY: typeof _PSEUDO_ELEMENT_PRIORITY = + _PSEUDO_ELEMENT_PRIORITY; export type InjectableStyle = _InjectableStyle; export type CompiledNamespaces = _CompiledNamespaces; diff --git a/packages/shared/src/preprocess-rules/index.js b/packages/shared/src/preprocess-rules/index.js index 060f7d6d..cd91d632 100644 --- a/packages/shared/src/preprocess-rules/index.js +++ b/packages/shared/src/preprocess-rules/index.js @@ -39,7 +39,6 @@ export default function flatMapExpandedShorthands( const expansion: ( string | number | null, ) => $ReadOnlyArray<[string, TStyleValue]> = - // $FlowFixMe[invalid-computed-prop] expansions[options.styleResolution ?? 'application-order'][key]; if (expansion) { if (Array.isArray(value)) { diff --git a/packages/shared/src/stylex-define-vars.js b/packages/shared/src/stylex-define-vars.js index 3bde1924..a6463354 100644 --- a/packages/shared/src/stylex-define-vars.js +++ b/packages/shared/src/stylex-define-vars.js @@ -74,7 +74,6 @@ export default function styleXDefineVars( ); const injectableStyles = constructCssVariablesString( - // $FlowFixMe[incompatible-call] - Can't use a class when a plain object is expected variablesMap, themeNameHash, ); diff --git a/packages/shared/src/utils/property-priorities.js b/packages/shared/src/utils/property-priorities.js index 3ffb3e96..08ba1467 100644 --- a/packages/shared/src/utils/property-priorities.js +++ b/packages/shared/src/utils/property-priorities.js @@ -749,7 +749,6 @@ export default function getPriority(key: string): number { ? key.slice(0, key.indexOf('(')) : key; - // $FlowFixMe[invalid-computed-prop] return PSEUDO_CLASS_PRIORITIES[prop] ?? 40; }