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

fix: incorrect jsx attribute name AST #425

Merged
merged 5 commits into from
Sep 7, 2022

Conversation

chenxsan
Copy link
Contributor

@chenxsan chenxsan commented Sep 6, 2022

I just noticed that ESLint failed to lint mdx now with the latest eslint-plugin-react here webpack/webpack.js.org#6377.

Here's an example mdx file:

---
title: hello
---

<div className='hi'>hello</div>

Here's the ast eslint-plugin-react would print for <div className='hi'>hello</div> node (https://github.com/jsx-eslint/eslint-plugin-react/blob/64ad60299d8c7d7534785c455694291b4c7aa427/lib/rules/no-unknown-property.js#L517):

<ref *1> {
  start: 27,
  end: 36,
  loc: {
    start: { line: 5, column: 5, offset: 27 },
    end: { line: 5, column: 14, offset: 36 }
  },
  range: [ 27, 36 ],
  raw: 'className',
  type: 'JSXAttribute',
  name: {
    start: 27,
    end: 41,
    loc: { start: [Object], end: [Object] },
    range: [ 27, 41 ],
    raw: "className='hi'",
    type: 'JSXIdentifier',
    name: 'className',
    parent: [Circular *1]
  },
  value: {
    start: 37,
    end: 41,
    loc: { start: [Object], end: [Object] },
    range: [ 37, 41 ],
    raw: "'hi'",
    type: 'Literal',
    value: 'hi',
    parent: [Circular *1]
  },
  parent: <ref *2> {
    type: 'JSXOpeningElement',
    name: {
      start: 23,
      end: 26,
      loc: [Object],
      range: [Array],
      raw: 'div',
      type: 'JSXIdentifier',
      name: 'div',
      parent: [Circular *2]
    },
    attributes: [ [Circular *1] ],
    selfClosing: false,
    start: 22,
    end: 42,
    loc: { start: [Object], end: [Object] },
    range: [ 22, 42 ],
    raw: "<div className='hi'>",
    parent: {
      start: 22,
      end: 53,
      loc: [Object],
      range: [Array],
      raw: "<div className='hi'>hello</div>",
      type: 'JSXElement',
      openingElement: [Circular *2],
      closingElement: [Object],
      children: [],
      parent: [Object]
    }
  }
}

The name part doesn't seem right to me, and it's causing an error:

$ npx eslint ./src/test.mdx
/Users/sam/Documents/githubRepos/webpack.js.org/src/test.mdx
  5:6  error  Unknown property 'className='hi'' found  react/no-unknown-property

✖ 1 problem (1 error, 0 warnings)

Actually I believe it's not really an issue in eslint-plugin-react, more like a bug in eslint-mdx parser which gets exposed by eslint-plugin-react's recent change. But I could be wrong since it's beyond my expertise :)

@changeset-bot
Copy link

changeset-bot bot commented Sep 6, 2022

🦋 Changeset detected

Latest commit: 2e86462

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
eslint-mdx Patch
eslint-plugin-mdx Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 6, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2022

📊 Package size report   No changes

File Before After
Total (Includes all files) 1.2 MB 1.2 MB
Tarball size 114.1 kB 114.1 kB
Unchanged files
File Size
.babelrc 35 B
.changeset/config.json 372 B
.changeset/README.md 510 B
.codesandbox/ci.json 76 B
.editorconfig 161 B
.eslintrc.js 591 B
.gitattributes 35 B
.github/FUNDING.yml 204 B
.github/workflows/ci.yml 941 B
.github/workflows/pkg-size.yml 496 B
.github/workflows/release.yml 914 B
.lintstagedrc.js 50 B
.simple-git-hooks.js 51 B
CHANGELOG.md 283 B
CONTRIBUTING.md 931 B
LICENSE 1.1 kB
package.json 2.5 kB
packages/eslint-mdx/CHANGELOG.md 21.0 kB
packages/eslint-mdx/package.json 1.2 kB
packages/eslint-mdx/README.md 9.8 kB
packages/eslint-mdx/shim.d.ts 1.4 kB
packages/eslint-mdx/src/helpers.ts 6.5 kB
packages/eslint-mdx/src/index.ts 98 B
packages/eslint-mdx/src/parser.ts 2.3 kB
packages/eslint-mdx/src/sync.ts 384 B
packages/eslint-mdx/src/tokens.ts 6.3 kB
packages/eslint-mdx/src/types.ts 1.2 kB
packages/eslint-mdx/src/worker.ts 19.2 kB
packages/eslint-mdx/tsconfig.json 131 B
packages/eslint-plugin-mdx/CHANGELOG.md 22.9 kB
packages/eslint-plugin-mdx/package.json 1.0 kB
packages/eslint-plugin-mdx/README.md 9.8 kB
packages/eslint-plugin-mdx/src/configs/base.ts 305 B
packages/eslint-plugin-mdx/src/configs/code-blocks.ts 1.0 kB
packages/eslint-plugin-mdx/src/configs/index.ts 345 B
packages/eslint-plugin-mdx/src/configs/overrides.ts 725 B
packages/eslint-plugin-mdx/src/configs/recommended.ts 806 B
packages/eslint-plugin-mdx/src/helpers.ts 620 B
packages/eslint-plugin-mdx/src/index.ts 105 B
packages/eslint-plugin-mdx/src/processors/helpers.ts 633 B
packages/eslint-plugin-mdx/src/processors/index.ts 244 B
packages/eslint-plugin-mdx/src/processors/options.ts 1.7 kB
packages/eslint-plugin-mdx/src/processors/remark.ts 1.5 kB
packages/eslint-plugin-mdx/src/processors/types.ts 241 B
packages/eslint-plugin-mdx/src/rules/.eslintrc 124 B
packages/eslint-plugin-mdx/src/rules/index.ts 138 B
packages/eslint-plugin-mdx/src/rules/remark.ts 3.3 kB
packages/eslint-plugin-mdx/src/rules/types.ts 415 B
packages/eslint-plugin-mdx/tsconfig.json 131 B
README.md 9.8 kB
test/__snapshots__/fixtures.test.ts.snap 21.7 kB
test/__snapshots__/parser.test.ts.snap 978.5 kB
test/fixtures.test.ts 1.6 kB
test/fixtures/287.mdx 204 B
test/fixtures/292.mdx 191 B
test/fixtures/334.mdx 70 B
test/fixtures/336.mdx 178 B
test/fixtures/367.mdx 87 B
test/fixtures/371.mdx 75 B
test/fixtures/380.mdx 11.0 kB
test/fixtures/391.mdx 62 B
test/fixtures/acorn.mdx 860 B
test/fixtures/adjacent.mdx 50 B
test/fixtures/async/.remarkrc 67 B
test/fixtures/basic.mdx 405 B
test/fixtures/basic.tsx 77 B
test/fixtures/blank-lines.mdx 768 B
test/fixtures/code-blocks.md 511 B
test/fixtures/comments.mdx 162 B
test/fixtures/details.mdx 881 B
test/fixtures/dir.mdx/.gitkeep 0 B
test/fixtures/jsx-in-list.mdx 810 B
test/fixtures/leading-spaces.mdx 42 B
test/fixtures/markdown.md 5 B
test/fixtures/no-unescaped-entities.mdx 99 B
test/fixtures/no-unused-expressions.mdx 90 B
test/fixtures/processor.mdx 13 B
test/fixtures/remark.md 171 B
test/fixtures/remark.mdx 169 B
test/fixtures/style/.remarkrc 228 B
test/fixtures/style/nested.md 10 B
test/fixtures/style/plugin1.cjs 26 B
test/fixtures/style/plugin2.mjs 24 B
test/fixtures/unicorn.jsx 119 B
test/fixtures/unicorn.mdx 118 B
test/helpers.test.ts 1.5 kB
test/helpers.ts 548 B
test/parser.test.ts 4.0 kB
test/remark.test.ts 2.8 kB
tsconfig.base.json 235 B
tsconfig.json 309 B
tsconfig.lib.json 180 B

🤖 This report was automatically generated by pkg-size-action

@codecov-commenter
Copy link

codecov-commenter commented Sep 6, 2022

Codecov Report

Merging #425 (62e9dad) into master (c8d38ee) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            master      #425   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines          196       196           
  Branches        40        40           
=========================================
  Hits           196       196           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@JounQin JounQin changed the title fix: fix name ast fix: incorrect jsx attribute name AST Sep 6, 2022
@JounQin
Copy link
Member

JounQin commented Sep 6, 2022

I'll do some investigation ASAP. ESLint compatible AST could really be different with the acorn's.


I don't remember why I wrote like that. 😅

@JounQin JounQin merged commit fb3f154 into mdx-js:master Sep 7, 2022
@chenxsan chenxsan deleted the bugfix/fix-ast branch September 8, 2022 00:59
JounQin added a commit that referenced this pull request Dec 10, 2023
JounQin added a commit that referenced this pull request Dec 11, 2023
JounQin added a commit that referenced this pull request Dec 12, 2023
JounQin added a commit that referenced this pull request Dec 20, 2023
JounQin added a commit that referenced this pull request Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants