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.

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

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

Conversation

javascripter
Copy link

@javascripter javascripter commented May 11, 2024

What changed / motivation ?

React Strict DOM recently removed their eslint plugin and I'm considering migrating to @stylexjs/eslint-plugin.

However, react-strict-dom exports stylex under css object, and it is it incompatible with current validImports option in the eslint plugin.

e.g. This doesn't work:

module.exports = {
  plugins: ['@stylexjs'],
  rules: {
    '@stylexjs/sort-keys': [
      'error',
      {
        validImports: ['react-strict-dom']
      },
    ],
  },
};

I have added { from: string, as: string } syntax similar to importSources option in the babel plugin for compatibility with RSD, for both @stylexjs/sort-keys and @stylexjs/valid-styles rules.

e.g. now the option accepts this format:

{
  validImports: [
    {
      from: 'react-strict-dom',
      as: 'css',
    },
  ],
},

Additional Context

  • Added unit tests
  • Updated the api docs
  • I also checked the behavior locally in apps/nextjs-example dir by replacing imports with react-strict-dom and configuring with the new options as in the screenshot.
Screenshot 2024-05-11 at 15 51 55

Pre-flight checklist

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 11, 2024
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".

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

@javascripter javascripter May 11, 2024

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.

});
}

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

@javascripter javascripter May 11, 2024

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'

@javascripter javascripter changed the title fix(@stylexjs/eslint-plugin): Accept { from: string, as: string } syntax in validImports option for RSD feat(@stylexjs/eslint-plugin): Accept { from: string, as: string } syntax in validImports option for RSD May 11, 2024
@necolas necolas requested a review from nmn May 21, 2024 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants