Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 71 additions & 75 deletions package-lock.json

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,78 @@ describe('@stylexjs/babel-plugin', () => {
}).toThrow(messages.INVALID_PSEUDO_OR_AT_RULE);
});

test('soft validation: invalid media query does not throw with softMediaQueryValidation enabled', () => {
const consoleWarnSpy = jest
.spyOn(console, 'warn')
.mockImplementation(() => {});

expect(() => {
transform(
`
import * as stylex from '@stylexjs/stylex';
const styles = stylex.create({
root: {
'color': {
'@media not ((not (min-width: 400px))': 'blue'
},
},
});
`,
{ enableMediaQueryOrder: true, softMediaQueryValidation: true },
);
}).not.toThrow();

expect(consoleWarnSpy).toHaveBeenCalledWith(
expect.stringContaining('Invalid media query syntax'),
);

consoleWarnSpy.mockRestore();
});

test('soft validation: invalid media query still throws without softMediaQueryValidation', () => {
expect(() => {
transform(
`
import * as stylex from '@stylexjs/stylex';
const styles = stylex.create({
root: {
'color': {
'@media not ((not (min-width: 400px))': 'blue'
},
},
});
`,
{ enableMediaQueryOrder: true, softMediaQueryValidation: false },
);
}).toThrow(messages.INVALID_MEDIA_QUERY_SYNTAX);
});

test('soft validation: valid media query does not warn', () => {
const consoleWarnSpy = jest
.spyOn(console, 'warn')
.mockImplementation(() => {});

expect(() => {
transform(
`
import * as stylex from '@stylexjs/stylex';
const styles = stylex.create({
root: {
'color': {
'@media (min-width: 768px)': 'blue'
},
},
});
`,
{ enableMediaQueryOrder: true, softMediaQueryValidation: true },
);
}).not.toThrow();

expect(consoleWarnSpy).not.toHaveBeenCalled();

consoleWarnSpy.mockRestore();
});

test('valid object value: key is "default"', () => {
expect(() => {
transform(`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export type StyleXOptions = $ReadOnly<{
enableDevClassNames?: ?boolean,
enableFontSizePxToRem?: ?boolean,
enableMediaQueryOrder?: ?boolean,
softMediaQueryValidation?: ?boolean,
enableLegacyValueFlipping?: ?boolean,
enableLogicalStylesPolyfill?: ?boolean,
enableLTRRTLComments?: ?boolean,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,17 @@ export function flattenRawStyleObject(
? lastMediaQueryWinsTransform(style)
: style;
} catch (error) {
throw new Error(messages.INVALID_MEDIA_QUERY_SYNTAX);
if (options.softMediaQueryValidation) {
console.warn(
`StyleX: ${messages.INVALID_MEDIA_QUERY_SYNTAX}\n` +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably don't need to "StyleX: " prefix here

'Media query order will not be respected. ' +
'This could be due to invalid media query syntax or unsupported edge cases in style-value-parser.\n' +
`Error: ${error.message || error}`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we validate that this error message contains the erroring media query?

);
processedStyle = style;
} else {
throw new Error(messages.INVALID_MEDIA_QUERY_SYNTAX);
}
}
return _flattenRawStyleObject(processedStyle, [], options);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export const defaultOptions: StyleXOptions = {
enableDebugDataProp: true,
enableFontSizePxToRem: false,
enableMediaQueryOrder: false,
softMediaQueryValidation: false,
enableLegacyValueFlipping: false,
enableLogicalStylesPolyfill: false,
enableLTRRTLComments: false,
Expand Down
10 changes: 10 additions & 0 deletions packages/@stylexjs/babel-plugin/src/utils/state-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ export type StyleXOptions = $ReadOnly<{
enableDevClassNames?: boolean,
enableInlinedConditionalMerge?: boolean,
enableMediaQueryOrder?: boolean,
softMediaQueryValidation?: boolean,
enableLegacyValueFlipping?: boolean,
enableLogicalStylesPolyfill?: boolean,
enableLTRRTLComments?: boolean,
Expand Down Expand Up @@ -254,6 +255,14 @@ export default class StateManager {
'options.enableMediaQueryOrder',
);

const softMediaQueryValidation: StyleXStateOptions['softMediaQueryValidation'] =
z.logAndDefault(
z.boolean(),
options.softMediaQueryValidation ?? false,
false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's make this true for now

'options.softMediaQueryValidation',
);

const enableLegacyValueFlipping: StyleXStateOptions['enableLegacyValueFlipping'] =
z.logAndDefault(
z.boolean(),
Expand Down Expand Up @@ -388,6 +397,7 @@ export default class StateManager {
enableInlinedConditionalMerge,
enableMinifiedKeys,
enableMediaQueryOrder,
softMediaQueryValidation,
enableLegacyValueFlipping,
enableLogicalStylesPolyfill,
enableLTRRTLComments,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

jest.disableAutomock();

const { RuleTester: ESLintTester } = require('eslint');
const rule = require('../src/stylex-transform-media-queries');

const eslintTester = new ESLintTester({
parser: require.resolve('hermes-eslint'),
parserOptions: {
ecmaVersion: 6,
sourceType: 'module',
},
});

eslintTester.run('stylex-transform-media-queries', rule.default, {
valid: [
{
code: `
import * as stylex from '@stylexjs/stylex';
const styles = stylex.create({
main: {
color: 'red',
},
})
`,
},
{
code: `
import * as stylex from '@stylexjs/stylex';
const styles = stylex.create({
main: {
color: {
default: 'yellow',
'@media (min-width: 768px)': 'blue',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we require a default with every contextual style:

Suggested change
'@media (min-width: 768px)': 'blue',
default: 'yellow',
'@media (min-width: 768px)': 'blue',

'@media (min-width: 1024px)': 'green',
},
},
})
`,
},
{
code: `
import * as stylex from '@stylexjs/stylex';
const styles = stylex.create({
main: {
color: {
'@media (max-width: 768px)': 'red',
default: 'blue',
},
},
})
`,
},
],
invalid: [
{
code: `
import * as stylex from '@stylexjs/stylex';
const styles = stylex.create({
main: {
color: {
'@media not ((not (min-width: 400px))': 'blue',
},
},
})
`,
errors: [
{
message:
/Media query order may not be respected\. Invalid media query syntax or unsupported edge cases in style-value-parser\. Error:/,
},
],
},
],
});
3 changes: 2 additions & 1 deletion packages/@stylexjs/eslint-plugin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
"dependencies": {
"css-shorthand-expand": "^1.2.0",
"micromatch": "^4.0.5",
"postcss-value-parser": "^4.2.0"
"postcss-value-parser": "^4.2.0",
"style-value-parser": "*"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe style-value-parser is already added to workspaces at the root level so it's inlined instead, as it's a private package, see: https://github.com/facebook/stylex/blob/main/package.json#L20

},
"files": [
"flow_modules/*",
Expand Down
3 changes: 3 additions & 0 deletions packages/@stylexjs/eslint-plugin/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import noUnused from './stylex-no-unused';
import sortKeys from './stylex-sort-keys';
import validShorthands from './stylex-valid-shorthands';
import validStyles from './stylex-valid-styles';
import transformMediaQueries from './stylex-transform-media-queries';

const rules: {
'enforce-extension': typeof enforceExtension,
Expand All @@ -25,6 +26,7 @@ const rules: {
'sort-keys': typeof sortKeys,
'valid-shorthands': typeof validShorthands,
'valid-styles': typeof validStyles,
'transform-media-queries': typeof transformMediaQueries,
} = {
'enforce-extension': enforceExtension,
'no-legacy-contextual-styles': noLegacyContextualStyles,
Expand All @@ -34,6 +36,7 @@ const rules: {
'sort-keys': sortKeys,
'valid-shorthands': validShorthands,
'valid-styles': validStyles,
'transform-media-queries': transformMediaQueries,
};

export { rules };
Loading