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

Parse error for multiline object props on React components #435

Closed
4 tasks done
mike-fam opened this issue Oct 5, 2022 · 16 comments · Fixed by #463
Closed
4 tasks done

Parse error for multiline object props on React components #435

mike-fam opened this issue Oct 5, 2022 · 16 comments · Fixed by #463
Assignees
Labels
🌊 blocked/upstream This cannot progress before something external happens first

Comments

@mike-fam
Copy link

mike-fam commented Oct 5, 2022

Initial checklist

Affected packages and versions

2.0.5

Link to runnable example

https://github.com/mike-fam/eslint-mdx-multiline-prop-error

Steps to reproduce

  1. Create a new workspace with npm and install eslint with eslint-plugin-mdx
  2. Eslint configuration:
{
   "overrides": [
   	{
   		"files": ["*.mdx"],
   		"extends": "plugin:mdx/recommended"
   	}
   ],
   "rules": {}
}
  1. Create an mdx file with a react component with a multiline object prop e.g.
<button
   style={{
   	display: 'block'
   }}
>
   Test
</button>
  1. Lint the file, note the error: Parsing error: Expected value to be truthy

Expected behavior

The file should pass linting without any problems, as this is valid mdx syntax.

Actual behavior

Linting produces the following error:
0:0 error Parsing error: Expected value to be truthy

Runtime

Node v16, Node v14

Package manager

yarn v1

OS

macOS

Build and bundle tools

webpack

@JounQin
Copy link
Member

JounQin commented Oct 5, 2022

Thanks for reporting issue by using the template, I'm on a vocation these days, I'll check later soon.

@JounQin JounQin self-assigned this Oct 5, 2022
@JounQin
Copy link
Member

JounQin commented Oct 9, 2022

It seems related to tab, if I change the source codes with spaces there will be no issue:

<button
  style={{
    display: "block",
  }}
>
  Test
</button>

The assertion error is thrown at

assert(text[attrValEnd] === '}')

I'll check how to fix this with tab


For text:

<button
	style={{
		display: 'block'
	}}
>
	Test
</button>

attrValue.data.estree.range is [16, 40], but it should be [16, 39] IMO, could it be an issue from micromark-extension-mdx-expression? @wooorm

I'll try to draft a simple reproduction for it.

@mike-fam
Copy link
Author

Hi @JounQin, thanks for all your great work and investigating this. Is there any new update on this? For now, do you recommend replacing all the tabs in our stories with spaces or waiting for a new fix to be released?

@JounQin JounQin added the 🌊 blocked/upstream This cannot progress before something external happens first label Oct 28, 2022
@morganfeeney
Copy link

Hey @mike-fam did you manage to work around this one? I'm facing the same thing.

@Ayc0
Copy link

Ayc0 commented Jan 14, 2023

I investigated a bit this issue.

With the following code:

-   list

    <Components data={`
    `} />

I also have the error Parsing error: Expected value to be truthy

This crashes because of this line:

assert(text[attrValEnd] === '}')
.

When adding console.logs before and after attrValStart = prevCharOffset(attrValStart - 1); attrValEnd = nextCharOffset(attrValEnd);, here is what I have:

image

I think that the reason of the offset is because when writing <Component data={`\n `} />, acorn returns this node:
image

But the offsets start & end on the data attribute (32, 35) don't match the right start & end (end should have been 32, 38):
image

I'm not sure why the spaces are dropped in this estree (which comes from this processor

export const getRemarkProcessor = async (
).

@mike-fam
Copy link
Author

mike-fam commented Mar 3, 2023

Hi @JounQin @wooorm sorry for bothering you. Is there any update on this issue?

@remcohaszing
Copy link
Member

If I’m correct, this repo is mainly maintained by @JounQin. However, we haven’t heard from him in a while.

Maybe we should create and pin a Looking for maintainers issue?

@JounQin
Copy link
Member

JounQin commented Apr 1, 2023

As I and @Ayc0 discovered above, it is not an internal issue but due to acorn return incorrect location info, but I'm not sure if it comes from acorn itself or micromark-extension-mdx-expression instead because the location info is restored at

https://github.com/micromark/micromark-extension-mdx-expression/blob/88c175f9ac684efc9306e23642807657df71d6a6/packages/micromark-util-events-to-acorn/dev/index.js#L216

cc @wooorm

@mike-fam
Copy link
Author

@JounQin @wooorm Would you guys mind creating an issue in either acorn or micromark-extension-mdx-expression, since I know little context to how the eslint-mdx plugin is implemented and how it uses those dependencies

@JounQin
Copy link
Member

JounQin commented May 4, 2023

@wooorm Sorry to ping again, but I think we need your help here.

@wooorm
Copy link
Member

wooorm commented May 4, 2023

I dunno where this problem is, I’d appreciate a reduced test case. If it isn’t in eslint-mdx, I’d appreciate a test case that doesn’t use eslint-mdx?

@JounQin
Copy link
Member

JounQin commented May 4, 2023

Working on a minimal reproduction without eslint-mdx.

@JounQin
Copy link
Member

JounQin commented May 4, 2023

@wooorm Please check https://github.com/JounQin/test/blob/mdx-tab/test.mjs#L29-L32

Besides, #435 (comment) could be another range issue not related to tab but list.


I tried acorn-jsx directly, there is no range issue:

/**
 * @typedef {import('estree').Program} Program
 * @typedef {import('estree-jsx').JSXElement} JSXElement
 * @typedef {import('estree-jsx').JSXAttribute} JSXAttribute
 */

const root =
  // @ts-expect-error
  /** @type {Program} */ (acorn.Parser.extend(acornJsx()).parse(contents, {
    ecmaVersion: 2020,
  }))
const statement = root.body[0]
if (typeof statement === 'object' && statement.type === 'ExpressionStatement') {
  const jsxElement = /** @type {JSXElement} */ (statement.expression)
  const attribute =
    /** @type {JSXAttribute} */ (jsxElement.openingElement.attributes[0])
  console.log(attribute.value)
}
Node {
  type: 'JSXExpressionContainer',
  start: 15,
  end: 21,
  expression: Node {
    type: 'TemplateLiteral',
    start: 16,
    end: 20,  // correct
    expressions: [],
    quasis: [ [Node] ]
  }
}

wooorm added a commit to micromark/micromark-extension-mdx-expression that referenced this issue May 13, 2023
@wooorm
Copy link
Member

wooorm commented May 17, 2023

@JounQin did you see my above commit?

@JounQin
Copy link
Member

JounQin commented May 17, 2023

@wooorm Thanks! It seems working to me now. @mike-fam @morganfeeney @Ayc0 Can you help to confirm? You may need to delete your npm-lock.json or yarn.lock, etc to upgrade all related packages to confirm.

@Ayc0
Copy link

Ayc0 commented May 17, 2023

I think that this is working for me too! Thank you very much! 🎉

(with yarn 2+, you can run yarn up -R micromark-util-events-to-acorn to upgrade micromark-util-events-to-acorn in sub-dependencies without having to remove the whole yarn.lock)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌊 blocked/upstream This cannot progress before something external happens first
Development

Successfully merging a pull request may close this issue.

6 participants