mdx-js / eslint-mdx

ESLint Parser/Plugin for MDX
https://npmjs.org/eslint-plugin-mdx
MIT License
259 stars 29 forks source link

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

Closed dimaMachina closed 7 months ago

dimaMachina commented 1 year ago

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
)
{'bar'} // 😬 nothing here ```` and ````mdx ```jsx let a = bar ❌ error Curly braces are unnecessary here react/jsx-curly-brace-presence ``` bar ❌ 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 commented 1 year ago

repro https://github.com/B2o5T/mdx-repro

u3u commented 7 months ago

Why is this happening? I'm not sure if there are any other rules that could cause strange issues, which makes me fearful of eslint-mdx. 😱

❌ Empty components are self-closing eslint(react/self-closing-comp)
<span>test</span>

💡 Fix this react/jsx-sort-props problem
<Image alt="alt" src="src" width="315" height="100" format="svg" />
↓↓↓↓↓
<Image alt="alt" format="src" height="315" src="100" width="svg" />
JounQin commented 7 months ago

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 commented 7 months ago

@JounQin There are serious issues when used with the react/self-closing-comp / react/jsx-sort-props rules.

  1. <span>test</span> Regarded as an empty tag self-closing, it will be fixed to <span />, directly losing the content.
  2. After automatic sorting of JSX attributes, the content and attributes no longer correspond.
JounQin commented 7 months ago

@u3u Please raise a new issue with online runnable reproduction, it seems a different issue from this one, and I've never met.

u3u commented 7 months ago

@JounQin Clone this repository https://github.com/u3u/eslint-config, and input the above content in any mdx file to reproduce.

JounQin commented 7 months ago

@u3u A new issue please, we must track different issues.

wooorm commented 7 months ago

<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 commented 7 months ago

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

react/self-closing-comp rule is referenced, so I said it's related to self-closing. Is that possible to add such property for mdx nodes?

wooorm commented 7 months ago

It shouldn’t be in the AST.

ASTs are about simplifying things. Not about presenting everything. Similar to how whether " or ' was used is not in ASTs. That’s what CSTs are.

JounQin commented 7 months ago

OK, let's skip this part.

JounQin commented 7 months ago

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 commented 7 months ago

@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 commented 7 months ago

v2.2.1 has already been released.