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

jsx-curly-spacing looks into functions #998

Closed
ColCh opened this issue Dec 12, 2016 · 3 comments
Closed

jsx-curly-spacing looks into functions #998

ColCh opened this issue Dec 12, 2016 · 3 comments

Comments

@ColCh
Copy link
Contributor

ColCh commented Dec 12, 2016

It produces such warning (or error, It's not important)

/foo/src/components/Bar.js
  75:93  error  There should be no space before '}'  react/jsx-curly-spacing

On this line:

onLayout={() => { /* dummy callback to fix android bug with component measuring */ }}

I've got to debug this rule, this is what I've noticed.
for this function:

isSpaceBetweenTokens(first, second) {
        const text = this.text.slice(first.range[1], second.range[0]);

        return /\s/.test(text.replace(/\/\*.*?\*\//g, ""));
    }

these arguments are passed:
first:

{
  "type": "Block",
  "value": " dummy callback to fix android bug with component measuring ",
  "start": 1722,
  "end": 1786,
  "loc": {
    "start": {
      "line": 75,
      "column": 26
    },
    "end": {
      "line": 75,
      "column": 90
    }
  },
  "range": [
    1722,
    1786
  ]
}

second:

{
  "type": "Punctuator",
  "value": "}",
  "start": 1788,
  "end": 1789,
  "loc": {
    "start": {
      "line": 75,
      "column": 92
    },
    "end": {
      "line": 75,
      "column": 93
    }
  },
  "range": [
    1788,
    1789
  ]
}

and text becomes --> " }" ... but that space belongs to function, not for jsx curly brace

version is [email protected]

@ljharb ljharb added the bug label Dec 13, 2016
@golopot
Copy link
Contributor

golopot commented Mar 17, 2019

I cannot reproduce this. Maybe it is fixed?

@ljharb
Copy link
Member

ljharb commented Mar 17, 2019

If so, we can close this with a PR containing the test case.

@ColCh
Copy link
Contributor Author

ColCh commented Mar 17, 2019

Such an ancient issue. Glad I've got package-lock and git repo to check this out 😃

TL, DR: Yes, bug is fixed in recent version. It would be nice to add test case for it to prevent future bugs

long

previous versions:

image

exact deps versions

  "dependencies": {
    "babel-eslint": "7.1.1",
    "eslint": "3.12.0",
    "eslint-plugin-react": "6.8.0"
  },

minimal amount of code to reproduce

<div onLayout={() => { /* dummy callback to fix android bug with component measuring */ }} />

and eslintrc

{
  "parser": "babel-eslint",
  "plugins": [
    "react"
  ],
  "rules": {
    "react/jsx-curly-spacing": [
      "error",
      "never",
      {
        "allowMultiline": true
      }
    ]
  }
}

using latest versions

deps

  "dependencies": {
    "babel-eslint": "10.0.1",
    "eslint": "5.15.2",
    "eslint-plugin-react": "7.12.4"
  },

tada, bug is gone 🎉

ColCh added a commit to ColCh/eslint-plugin-react that referenced this issue Mar 17, 2019
ljharb pushed a commit to ColCh/eslint-plugin-react that referenced this issue Mar 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants