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

Strange behaviour of react/jsx-curly-brace-presence/react/self-closing-comp #437

Closed
4 tasks done
dimaMachina opened this issue Oct 31, 2022 · 14 comments · Fixed by #493
Closed
4 tasks done

Strange behaviour of react/jsx-curly-brace-presence/react/self-closing-comp #437

dimaMachina opened this issue Oct 31, 2022 · 14 comments · Fixed by #493
Assignees
Labels
🐛 type/bug This is a problem 🙆 yes/confirmed This is confirmed and ready to be worked on

Comments

@dimaMachina
Copy link
Contributor

dimaMachina commented Oct 31, 2022

Initial checklist

Affected packages and versions

2.0.5

Link to runnable example

No response

Steps to reproduce

// test.mdx

```jsx
let a = (
  <App prop="foo">{'bar'}</App> // ❌ error  Curly braces are unnecessary here  react/jsx-curly-brace-presence
)
```

<App prop="foo">{'bar'}</App> // 😬 nothing here

and

```jsx
let a = <App prop={'foo'}>bar</App> ❌ error  Curly braces are unnecessary here  react/jsx-curly-brace-presence
```

<App prop={'foo'}>bar</App> ❌ error  Curly braces are unnecessary here  react/jsx-curly-brace-presence / error  Empty components are self-closing  react/self-closing-comp

Expected behavior

should report only react/jsx-curly-brace-presence errors

Actual behavior

reports react/jsx-curly-brace-presence and react/self-closing-comp

Runtime

No response

Package manager

No response

OS

No response

Build and bundle tools

No response

@dimaMachina
Copy link
Contributor Author

@u3u

This comment has been minimized.

@JounQin
Copy link
Member

JounQin commented Dec 9, 2023

This happens because we don't know whether it's self-closing from mdx AST.

cc @wooorm


@u3u I don't know what's your meaning.

@u3u

This comment has been minimized.

@JounQin

This comment has been minimized.

@u3u

This comment has been minimized.

@JounQin

This comment was marked as off-topic.

u3u added a commit to u3u/eslint-config that referenced this issue Dec 9, 2023
@wooorm
Copy link
Member

wooorm commented Dec 9, 2023

<App prop="foo">{'bar'}</App> // 😬 nothing here

MDX is not JSX. There is a difference. <x>*a*</x> vs <x>{'*a*'}</x> are different.

This happens because we don't know whether it's self-closing from mdx AST.

Indeed. The AST is supposed to be simple and changeable.

What has self-closing to do with this example? It’s not a self-closing JSX element?

@JounQin

This comment was marked as off-topic.

@wooorm

This comment was marked as off-topic.

@JounQin

This comment was marked as off-topic.

@JounQin
Copy link
Member

JounQin commented Dec 9, 2023

What has self-closing to do with this example? It’s not a self-closing JSX element?

I think that's because the jsx content is not included or incorrect in the AST, so react/self-closing-comp rule does not recognize the jsx content correctly.

MDX is not JSX. There is a difference. a vs {'a'} are different.

Similar to react/self-closing-comp rule and AST difference mentioned above, react/jsx-curly-brace-presence does not recognize the content correctly.


I'll take a look how can I fix them.

JounQin added a commit that referenced this issue Dec 9, 2023
JounQin added a commit that referenced this issue Dec 10, 2023
@JounQin
Copy link
Member

JounQin commented Dec 10, 2023

@dimaMachina @u3u

Please help to test the part of react/jsx-curly-brace-presence andreact/self-closing-comp.

# yarn 1
yarn add https://pkg.csb.dev/mdx-js/eslint-mdx/commit/c3d7aedb/eslint-mdx https://pkg.csb.dev/mdx-js/eslint-mdx/commit/c3d7aedb/eslint-plugin-mdx
# yarn 2, 3
yarn add eslint-mdx@https://pkg.csb.dev/mdx-js/eslint-mdx/commit/c3d7aedb/eslint-mdx/_pkg.tgz eslint-plugin-mdx@https://pkg.csb.dev/mdx-js/eslint-mdx/commit/c3d7aedb/eslint-plugin-mdx/_pkg.tgz
# npm
npm i https://pkg.csb.dev/mdx-js/eslint-mdx/commit/c3d7aedb/eslint-mdx https://pkg.csb.dev/mdx-js/eslint-mdx/commit/c3d7aedb/eslint-plugin-mdx

@JounQin JounQin self-assigned this Dec 11, 2023
@JounQin JounQin added 🐛 type/bug This is a problem 🙆 yes/confirmed This is confirmed and ready to be worked on labels Dec 11, 2023
JounQin added a commit that referenced this issue Dec 20, 2023
JounQin added a commit that referenced this issue Dec 21, 2023
@JounQin
Copy link
Member

JounQin commented Dec 21, 2023

v2.2.1 has already been released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 type/bug This is a problem 🙆 yes/confirmed This is confirmed and ready to be worked on
Development

Successfully merging a pull request may close this issue.

4 participants