no-unused-vars-before-return does not detect use as React element

[rmccue]

The following code is incorrectly triggering a no-unused-vars-before-return error:

function () {
    const ItemElement = withFilters( 'foo' )( MyElement );
    return (
        <ItemElement />
    );
}

I suspect the rule is failing to detect the React element as usage of the variable.

[jorgefilipecosta]

Thank you for reporting this issue @rmccue.
It seems variable.references does not include JSX component references. The other explanation would be that the item is used after the line where return appears but inside return but that's not the case as the following code does not trigger the error:

function () {
    const ItemElement = withFilters( 'foo' )( MyElement );
    return (
        ItemElement
    );
}

I guess the way to fix the problem, would be to inside the loop that that iterates on the variables:
https://github.com/WordPress/gutenberg//blob/b9e9aedda252e91bcecf3a34a19cf8af79bbdef2/packages/eslint-plugin/rules/no-unused-vars-before-return.js#L18

Query all JSX element tags with name equal to that variable JSXOpeningElement[name.name=\'VariableNAME\'] and verify if one is before the return. Any thoughts on this approach @aduth?

[aduth]

In revisiting the logic, I see it does not consider the use in the JSX as a "reference" at this line:

https://github.com/WordPress/gutenberg//blob/b9e9aedda252e91bcecf3a34a19cf8af79bbdef2/packages/eslint-plugin/rules/no-unused-vars-before-return.js#L39

To me, this seems like an error in the parser, as quite clearly it is a reference to the variable.

Two thoughts:

• As you mention, if there was some way to query for JSX tags specifically, we could also check for this as part of considering whether there is a reference. In this context, however, I'm not sure "querying" is possible in this way, and it might need to be formed instead as a separate visitor property in the rule.
• If it were possible to, try to force the transpilation effect of the JSX to take effect before the ESLint rule is run, since it's likely to work better in its createElement( ItemElement ) form

[aduth]

I'm not sure it would have any impact, but it would be worth some quick testing to see whether bumping dependencies versions would make any difference here, if indeed it's an issue with the parse result. Specifically thinking one of babel-eslint, @babel/core, @babel/plugin-syntax-jsx.

[tn3rb]

related: eslint/eslint#12117

according to that issue, babel-eslint just needs to be bumped to 10.0.3

@wordpress/eslint-plugin currently lists "babel-eslint": "^10.0.2" as a dependency

[swissspidy]

If I am not mistaken this is working as expected nowadays.

[aduth]

Testing this today, it appears to still report a false positive.

Per #16418 (comment), we are now running ^10.0.3 as our babel-eslint dependency. It's not clear to me whether eslint/eslint#12117 is in-fact related to the issue here.

[aduth]

Fix at #21358 seeks to account for this manually.