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

docs: add clarification on stylistic overrides and Svelte overrides #446

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jacob-8
Copy link

@jacob-8 jacob-8 commented Apr 12, 2024

Description

Adds a little instruction on how to apply overrides. These are things I had to figure out the hard way and hopefully can save the next guy some time by these tips.

Feel free to ignore these tips if they bloat the readme, but it's just what would have helped me out. :)

On the note of the stylistic overrides, I first used the stylistic function to provide overrides, but that bit me when style/spaced-comment was now also applied to yaml files instead of being overwritten by the yaml specific version. Thank goodness for the config inspector 😁.

image

On the note of Svelte, I personally don't think a simple svelte: true is usable at all. I have about 18 overrides. Some of them are preferences and nice things, but others are required just to make things work for Svelte, because there are some general rules that conflict. For example no-self-assign clashes with the way you must reassign an array to itself to get reactivity updates (this is going away in Svelte 5, but exists until then).

Note that I use two extensions for Svelte files because of Kitbook compositions. I obviously didn't put the extra extension in my example here but I believe for Typescript to apply to the Svelte script block it needs done as shown here. Please correct if I'm too verbose but this is what works for me.

Copy link

netlify bot commented Apr 12, 2024

Deploy Preview for melodious-froyo-4871f8 ready!

Name Link
🔨 Latest commit 6538dc5
🔍 Latest deploy log https://app.netlify.com/sites/melodious-froyo-4871f8/deploys/6619347f38ba530007e804f8
😎 Deploy Preview https://deploy-preview-446--melodious-froyo-4871f8.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

README.md Outdated
Comment on lines 471 to 479
typescript({
componentExts: ['svelte'],
}),
svelte({
typescript: true,
overrides: {
'svelte/no-dom-manipulating': 'error',
},
}),
Copy link
Owner

Choose a reason for hiding this comment

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

I would not suggest users to directly use those sub configs manually to overrides,

  • It's easy to duplicate configs, for example, here typescript will be created twice with the order messed up
  • You need to know very well how each config work, and duplicated the information like having svelte in ts, and ts in svelte. Could be hard to keep track on it.

Instead, I'd recommend to use the generalized composer: https://github.com/antfu/eslint-config#config-composer

Where you can override, or replace an entire config item as you want.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I updated it to accomplish my overrides via the config composer option so now the Typescript stuff is not duplicated. The Svelte stuff was never duplicated the old way but perhaps it's best to keep it uniform by doing all the overrides this way. Did I do it right this time? 😄

So you can see my full example, using the strategy suggested in my pr here, I now have this:

import { antfu } from '@antfu/eslint-config'
import svelteStylistic from 'eslint-plugin-svelte-stylistic'

export default antfu(
  {
    ignores: [
      '**/.svelte-kit**',
      '.eslintcache',
      '**/route/kitbook/**',
    ],
    stylistic: {
      overrides: {
        'style/brace-style': ['error', '1tbs', { allowSingleLine: true }],
      },
    },
    svelte: true,
  },
  {
    name: 'jacob/svelte/stylistic',
    files: ['**/*.svelte', '**/*.composition'],
    plugins: {
      'svelte-stylistic': svelteStylistic,
    },
    rules: {
      'svelte-stylistic/brackets-same-line': 'error',
      'svelte-stylistic/consistent-attribute-lines': 'error',
    },
  },
  {
    name: 'jacob/test/rules',
    files: ['**/*.test.ts'],
    rules: {
      'test/consistent-test-it': ['error', { fn: 'test' }],
      'test/no-commented-out-tests': 'error',
      'test/no-disabled-tests': 'error',
      'test/consistent-test-filename': 'error',
      'test/expect-expect': 'error',
      'test/no-alias-methods': 'error',
      'test/no-conditional-expect': 'error',
      'test/no-conditional-in-test': 'error',
      'test/no-conditional-tests': 'error',
      'test/no-duplicate-hooks': 'error',
      'test/no-focused-tests': 'error',
      'test/no-standalone-expect': 'error',
      'test/no-test-return-statement': 'error',
      'test/prefer-comparison-matcher': 'error',
      'test/prefer-hooks-on-top': 'error',
      'test/prefer-spy-on': 'error',
      'test/prefer-to-be-falsy': 'error',
      'test/prefer-to-be-truthy': 'error',
      'test/prefer-to-contain': 'error',
      'test/prefer-to-have-length': 'error',
      'test/valid-describe-callback': 'error',
      'test/valid-expect': 'error',
    },
  },
).overrides({
  'antfu/typescript/rules': {
    files: ['**/*.svelte', '**/*.composition'],
    rules: {
      'prefer-destructuring': 'error',
      'no-constant-binary-expression': 'error',
      'ts/default-param-last': 'error',
      'require-await': 'error',
      'prefer-object-spread': 'error',
      'no-useless-concat': 'error',
      'no-else-return': 'error',
      'no-console': ['error', { allow: ['warn', 'error', 'info', 'time', 'timeEnd'] }],
      'require-atomic-updates': 'error',
      'style/quotes': ['error', 'single', {
        allowTemplateLiterals: true,
        avoidEscape: true,
      }],
      'ts/no-unused-vars': ['warn', {
        argsIgnorePattern: '^_',
        caughtErrors: 'none',
        ignoreRestSiblings: true,
        varsIgnorePattern: '^\\$\\$Props$',
      }],
      'ts/no-explicit-any': 'warn',
      'prefer-named-capture-group': 'warn',
      'no-undef': 'off',
      'no-unused-vars': 'off',
      'curly': 'off',
      'no-alert': 'off',
      'antfu/if-newline': 'off',
      'ts/ban-ts-comment': 'off',
      'ts/sort-type-constituents': 'off',
    },
  },
  'antfu/svelte/rules': {
    files: ['**/*.composition'],
    overrides: {
      'svelte/valid-compile': ['error', { ignoreWarnings: true }], // because this throws numerous useless a11y errors
      'svelte/no-dom-manipulating': 'error',
      'svelte/no-store-async': 'error',
      'svelte/require-store-reactive-access': 'error',
      'svelte/require-event-dispatcher-types': 'error',
      'svelte/button-has-type': 'error',
      'svelte/no-extra-reactive-curlies': 'error',
      'svelte/mustache-spacing': 'error',
      'svelte/html-closing-bracket-spacing': 'error',
      'svelte/no-reactive-reassign': ['warn', { props: false }],
      'svelte/html-quotes': 'off',
      'svelte/no-at-html-tags': 'off',
      'no-unused-expressions': 'off',
      'no-inner-declarations': 'off',
      'style/space-infix-ops': 'off',
      'no-undef-init': 'off',
      'no-self-assign': 'off',
    },
  },
})

Copy link
Author

Choose a reason for hiding this comment

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

As a bit of general feedback on your docs, it's hard to keep track of and know how to use the different overrides options. For vue and typescript (but not svelte) you can provide them directly in the integrations:

vue: {
  overrides: {
    'vue/operator-linebreak': ['error', 'before'],
  },
},
typescript: {
  overrides: {
    'ts/consistent-type-definitions': ['error', 'interface'],
  },
},

You can provide them as a new ruleset, and then finally using the Config Composer. The section on Config Composer I completely didn't understand the first time reading through. A tip there about getting names from the config inspector would be helpful.

Now finally, after reflecting on everything it seems that if I didn't have my special .composition extension, I could have just done things like this:

export default antfu(
  {
    svelte: true,
  },
  {
    files: ['**/*.svelte'],
    rules: {
      'svelte/no-dom-manipulating': 'error',
    },
  },
).overrides({
  'antfu/typescript/rules': {
    files: ['**/*.svelte'],
    rules: {
      'prefer-destructuring': 'error',
    },
  },
})

Want me to use that method in my doc update instead?

},
},
'antfu/typescript/rules': {
files: ['**/*.svelte'],
Copy link
Owner

Choose a reason for hiding this comment

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

This would break support for other ts files. I guess it's better to append a separate config that overrides that rule for only **/*.svelte but not other ts files.

Copy link
Author

@jacob-8 jacob-8 Apr 30, 2024

Choose a reason for hiding this comment

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

Sorry to be slow. Why would that break support? It only adds **/*.svelte to the already existing files array. It's working great for me in multiple projects.

@thenbe
Copy link
Contributor

thenbe commented May 22, 2024

@jacob-8 You can customize your rules like this. Would that work for you? Or have I misunderstood your use case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants