Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(@stylexjs/eslint-plugin): Accept { from: string, as: string } syntax in validImports option for RSD #569

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+177 −53
Open

feat(@stylexjs/eslint-plugin): Accept { from: string, as: string } syntax in validImports option for RSD #569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a5cb2a
feat: Allow eslint plugins to accept object syntax in validImports
javascripter May 8, 2024
ff87cd0
Merge branch 'main' into eslint-plugin-valid-imports-allow-objects
nmn Jun 23, 2024
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
4 changes: 2 additions & 2 deletions apps/docs/docs/api/configuration/eslint-plugin.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type Options = {
// Possible strings where you can import stylex from
//
// Default: ['@stylexjs/stylex']
validImports: Array<string>,
validImports: Array<string | { from: string, as: string }>,

// Custom limits for values of various properties
propLimits?: PropLimits,
Expand Down Expand Up @@ -99,7 +99,7 @@ type Options = {
// Possible string where you can import stylex from
//
// Default: ['@stylexjs/stylex']
validImports: Array<string>,
validImports: Array<string | { from: string, as: string }>,

// Minimum number of keys required after which the rule is enforced
//
Expand Down
41 changes: 41 additions & 0 deletions packages/eslint-plugin/__tests__/stylex-sort-keys-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,18 @@ eslintTester.run('stylex-sort-keys', rule.default, {
});
`,
},
{
options: [{ validImports: [{ from: 'a', as: 'css' }] }],
code: `
import { css } from 'a';
const styles = css.create({
button: {
borderColor: 'black',
display: 'flex',
}
});
`,
},
{
code: `
import { keyframes } from 'stylex';
Expand Down Expand Up @@ -672,5 +684,34 @@ eslintTester.run('stylex-sort-keys', rule.default, {
},
],
},
{
options: [{ validImports: [{ from: 'a', as: 'css' }] }],
code: `
import { css } from 'a';
const styles = css.create({
main: {
padding: 10,
animationDuration: '100ms',
fontSize: 12,
}
});
`,
output: `
import { css } from 'a';
const styles = css.create({
main: {
animationDuration: '100ms',
padding: 10,
fontSize: 12,
}
});
`,
errors: [
{
message:
'StyleX property key "animationDuration" should be above "padding"',
},
],
},
],
});
20 changes: 17 additions & 3 deletions packages/eslint-plugin/__tests__/stylex-valid-styles-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1387,7 +1387,23 @@ This property is not supported in legacy StyleX resolution.`,
},
{
code: `
import stylex from'stylex';
import { css } from 'a';
const styles = css.create({
base:{
background: ''
},
});
`,
options: [{ validImports: [{ from: 'a', as: 'css' }] }],
errors: [
{
message: 'The empty string is not allowed by Stylex.',
},
],
},
{
code: `
import stylex from'stylex';
const styles = stylex.create({
b:{
textUnderlineOffset: '',
Expand All @@ -1406,8 +1422,6 @@ initial
inherit
unset
revert`,
},
],
},
],
});
82 changes: 57 additions & 25 deletions packages/eslint-plugin/src/stylex-sort-keys.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@ import getPropertyPriorityAndType from './utils/getPropertyPriorityAndType';
/*:: import { Rule } from 'eslint'; */

type Schema = {
validImports: Array<string>,
validImports: Array<
| string
| {
from: string,
as: string,
},
>,
minKeys: number,
allowLineSeparatedGroups: boolean,
};
Expand Down Expand Up @@ -64,7 +70,18 @@ const stylexSortKeys = {
properties: {
validImports: {
type: 'array',
items: { type: 'string' },
items: {
oneOf: [
{ type: 'string' },
{
type: 'object',
properties: {
from: { type: 'string' },
as: { type: 'string' },
},
},
],
},
default: ['stylex', '@stylexjs/stylex'],
},
minKeys: {
Expand Down Expand Up @@ -128,32 +145,47 @@ const stylexSortKeys = {
return;
}

if (!importsToLookFor.includes(node.source.value)) {
return;
}

node.specifiers.forEach((specifier) => {
if (
specifier.type === 'ImportDefaultSpecifier' ||
specifier.type === 'ImportNamespaceSpecifier'
) {
styleXDefaultImports.add(specifier.local.name);
const foundImportSource = importsToLookFor.find((importSource) => {
Copy link
Author

Choose a reason for hiding this comment

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

foundImportSource will be either a string or the object containing "as" and "from".

if (typeof importSource === 'string') {
return importSource === node.source.value;
}
return importSource.from === node.source.value;
});

if (
specifier.type === 'ImportSpecifier' &&
specifier.imported.name === 'create'
) {
styleXCreateImports.add(specifier.local.name);
}
if (typeof foundImportSource === 'string') {
Copy link
Author

@javascripter javascripter May 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Then, if foundImportSource is a string,styleXDefaultImports and the other variables will be updated exactly the same as before.

node.specifiers.forEach((specifier) => {
if (
specifier.type === 'ImportDefaultSpecifier' ||
specifier.type === 'ImportNamespaceSpecifier'
) {
styleXDefaultImports.add(specifier.local.name);
}

if (
specifier.type === 'ImportSpecifier' &&
specifier.imported.name === 'keyframes'
) {
styleXKeyframesImports.add(specifier.local.name);
}
});
if (
specifier.type === 'ImportSpecifier' &&
specifier.imported.name === 'create'
) {
styleXCreateImports.add(specifier.local.name);
}

if (
specifier.type === 'ImportSpecifier' &&
specifier.imported.name === 'keyframes'
) {
styleXKeyframesImports.add(specifier.local.name);
}
});
}

if (typeof foundImportSource === 'object') {
Copy link
Author

@javascripter javascripter May 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If using the new as and from syntax, we will treat the value of the as option as the default import name. This is because the resolution mechanism is the same as import stylex from 'xxx', where the property names of stylex are fixed to create, props, etc, and the same constraints apply to the css object as well.

import { css } from 'a'

node.specifiers.forEach((specifier) => {
if (specifier.type === 'ImportSpecifier') {
if (specifier.imported.name === foundImportSource.as) {
styleXDefaultImports.add(specifier.local.name);
}
}
});
}
},
CallExpression(
node: $ReadOnly<{ ...CallExpression, ...Rule.NodeParentExtension }>,
Expand Down
83 changes: 60 additions & 23 deletions packages/eslint-plugin/src/stylex-valid-styles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2280,7 +2280,18 @@ const stylexValidStyles = {
properties: {
validImports: {
type: 'array',
items: { type: 'string' },
items: {
oneOf: [
{ type: 'string' },
{
type: 'object',
properties: {
from: { type: 'string' },
as: { type: 'string' },
},
},
],
},
default: ['stylex', '@stylexjs/stylex'],
},
allowOuterPseudoAndMedia: {
Expand Down Expand Up @@ -2336,7 +2347,13 @@ const stylexValidStyles = {
};

type Schema = {
validImports: Array<string>,
validImports: Array<
| string
| {
from: string,
as: string,
},
>,
allowOuterPseudoAndMedia: boolean,
banPropsForLegacy: boolean,
propLimits?: PropLimits,
Expand Down Expand Up @@ -2801,34 +2818,54 @@ const stylexValidStyles = {
return;
}
const sourceValue = node.source.value;
const isStylexImport = importsToLookFor.includes(sourceValue);

const foundImportSource = importsToLookFor.find((importSource) => {
if (typeof importSource === 'string') {
return importSource === sourceValue;
}
return importSource.from === sourceValue;
});

const isStylexImport = foundImportSource !== undefined;
const isStylexDefineVarsImport = sourceValue.endsWith(
stylexDefineVarsFileExtension,
);
if (!(isStylexImport || isStylexDefineVarsImport)) {
return;
}
if (isStylexImport) {
node.specifiers.forEach((specifier) => {
if (
specifier.type === 'ImportDefaultSpecifier' ||
specifier.type === 'ImportNamespaceSpecifier'
) {
styleXDefaultImports.add(specifier.local.name);
}
if (
specifier.type === 'ImportSpecifier' &&
specifier.imported.name === 'create'
) {
styleXCreateImports.add(specifier.local.name);
}
if (
specifier.type === 'ImportSpecifier' &&
specifier.imported.name === 'keyframes'
) {
styleXKeyframesImports.add(specifier.local.name);
}
});
if (typeof foundImportSource === 'string') {
node.specifiers.forEach((specifier) => {
if (
specifier.type === 'ImportDefaultSpecifier' ||
specifier.type === 'ImportNamespaceSpecifier'
) {
styleXDefaultImports.add(specifier.local.name);
}
if (
specifier.type === 'ImportSpecifier' &&
specifier.imported.name === 'create'
) {
styleXCreateImports.add(specifier.local.name);
}
if (
specifier.type === 'ImportSpecifier' &&
specifier.imported.name === 'keyframes'
) {
styleXKeyframesImports.add(specifier.local.name);
}
});
}

if (typeof foundImportSource === 'object') {
node.specifiers.forEach((specifier) => {
if (specifier.type === 'ImportSpecifier') {
if (specifier.imported.name === foundImportSource.as) {
styleXDefaultImports.add(specifier.local.name);
}
}
});
}
}

if (isStylexDefineVarsImport) {
Expand Down
Loading