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

Autofix for grid-column shorthand uses wrong syntax #6918

Closed
sortingbubbles opened this issue Jun 13, 2023 · 3 comments · Fixed by #6957
Closed

Autofix for grid-column shorthand uses wrong syntax #6918

sortingbubbles opened this issue Jun 13, 2023 · 3 comments · Fixed by #6957
Labels
status: wip is being worked on by someone type: bug a problem with a feature or rule

Comments

@sortingbubbles
Copy link

What steps are needed to reproduce the bug?

In a vue project that has stylelint use the long syntax for grid-column and try to use stylelint --fix to correct it.

What Stylelint configuration is needed to reproduce the bug?

Our project's stylelint configuration:

module.exports = {
  extends: '@company/stylelint-plugin',
  rules: {
    'selector-pseudo-element-no-unknown': [true, { ignorePseudoElements: ['v-deep'] }],
    'selector-class-pattern': null,
    'font-weight-notation': 'numeric',
    'declaration-block-no-redundant-longhand-properties': [
      true,
      { ignoreShorthands: ['grid-column'] },
    ],
    'declaration-property-value-disallowed-list': [
      {
        'font-size': [
          'xx-small',
          'x-small',
          'small',
          'medium',
          'large',
          'x-large',
          'xx-large',
          'xxx-large',
          'smaller',
          'larger',
        ],
        'line-height': ['normal'],
      },
      {
        message: (prop) => {
          switch (prop) {
            case 'font-size':
            case 'line-height':
              return 'Keyword values are not allowed'
          }
        },
        severity: 'error',
      },
    ],
  },
}

Our reusable, @company/stylelint-plugin, stylelint config

module.exports = {
  extends: [
    'stylelint-config-idiomatic-order',
    'stylelint-config-standard-scss',
    'stylelint-config-standard-vue/scss',
  ],
  defaultSeverity: 'error',
  rules: {
    indentation: 2,
    'color-no-hex': true,
    'color-hex-case': 'lower',
    'color-hex-length': 'short',
    'color-named': 'never',
    'font-weight-notation': 'numeric',
    'unit-allowed-list': ['%', 'fr', 'vw', 'vh', 's', 'ms', 'deg', 'px'],
    'function-disallowed-list': ['rgba', 'lighten', 'darken', 'rgb'],
    'number-max-precision': 3,
    'number-leading-zero': 'always',
    'number-no-trailing-zeros': true,
    'selector-max-id': 0,
    'selector-no-qualifying-type': true,
    'max-nesting-depth': 4,
    'value-keyword-case': 'lower',
    'string-quotes': 'single',
    'selector-max-compound-selectors': 3,
    'scss/dollar-variable-colon-space-after': 'at-least-one-space',
    'declaration-no-important': true,
  },
}

How did you run Stylelint?

By running lint-staged:

  "lint-staged": {
    "*.{js,ts,vue}": "eslint --ext .js,.ts,.vue --fix",
    "*.{scss,vue}": "stylelint --fix"
  },

Which version of Stylelint or dependencies are you using?

    "stylelint": "^15.6.2",
    "stylelint-config-recommended-scss": "^8.0.0",
    "stylelint-config-standard-scss": "^7.0.0",

What did you expect to happen?

The following rules:

&:deep(> *) {
  grid-column-end: 11;
  grid-column-start: 3;
}

to be transformed to

&:deep(> *) {
  grid-column: 3 / 11;
}

What actually happened?

The following rule:

&:deep(> *) {
  grid-column-end: 11;
  grid-column-start: 3;
}

was transformed to this:

&:deep(> *) {
  grid-column: 3 11;
}

Does the bug relate to non-standard syntax?

No, it's related to applying the standard syntax for grid-column inside a Vue component!

Proposal to fix the bug

No response

@ybiquitous
Copy link
Member

@sortingbubbles Thanks for the report with details. I doubt this is likely a bug of some rule, but I need help identifying the cause. Can you please provide a minimal reproduction with our demo site?

@ybiquitous ybiquitous added the status: needs reproducible example triage needs reproducible demo or repo label Jun 13, 2023
@mattxwang
Copy link
Member

Just to chime in - I'm guessing this is related to the other incorrect autofix issues with the declaration-block-no-redundant-longhand-properties autofix I introduced earlier? With a brief glance at the spec, the simple ordering logic that we use in the rule is incorrect.

Similar to the other autofixes related to the rule, I'm happy to help contribute (especially as I'll have much more time to work on Stylelint starting next week). I'll wait on a reproducible example before then!

@mattxwang
Copy link
Member

Here's a minimum reproducible demo; the autofix should be

a {
	grid-column: 1 / 2;
}

a {
	grid-row: 1 / 2;
}

instead of

a {
	grid-column: 1 2;
}

a {
	grid-row: 1 2;
}

@mattxwang mattxwang added status: wip is being worked on by someone type: bug a problem with a feature or rule and removed status: needs reproducible example triage needs reproducible demo or repo labels Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone type: bug a problem with a feature or rule
3 participants