-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Breaking change from 7.25.1 to 7.25.2: incorrect no-unused-vars
error
#3080
Comments
This reverts 3b83b6b because version 7.25.2 causes issues with FBT tags, see: jsx-eslint/eslint-plugin-react#3080
This is actually correct - lowercase tag names, by jsx spec, never point to a value, and only are used as HTML tags. In other words, if you delete that import, your code should work identically. |
I understand what you are saying. Unfortunately, FBT is a bit weird and doesn't work without the import. For example, I deleted it from the file I mentioned in my issue above and this is what the FBT Collect script throws:
Removing the import makes the translations broken. 😞 EDIT: the same happens in runtime, not only when collecting the translations. So the whole application doesn't work without the import. |
Right, but what if you do |
This reverts 3b83b6b because version 7.25.2 causes issues with FBT tags, see: jsx-eslint/eslint-plugin-react#3080
This reverts 3b83b6b3ec610b5d240b933be4c953e5d0e62733 because version 7.25.2 causes issues with FBT tags, see: jsx-eslint/eslint-plugin-react#3080 adeira-source-id: c30973cfbb659a5c7327e8c66ec7c2d662b84bb5
Hmm - does that mean you’re using a non-standard jsx transform? |
I am afraid that's exactly how FBT works, yes. Say we have the following minimal example: import fbt from 'fbt';
function MyComponent() {
return <fbt desc="…">test</fbt>;
} It would result in the following code only with import fbt from 'fbt';
function MyComponent() {
return /*#__PURE__*/React.createElement("fbt", {
desc: "\u2026"
}, "test");
} However, together with import fbt from 'fbt';
function MyComponent() {
return fbt._("__FBT__{\"type\":\"text\",\"jsfbt\":\"test\",\"desc\":\"\u2026\",\"project\":\"\"}__FBT__");
} As described here: https://facebook.github.io/fbt/docs/transform |
In that case, i think just like this plugin had “jsx uses react” as a rule, that marks things as used, fbt needs its own rule for this also (not inside this plugin, I’d think). It should be pretty easy to modify the jsx-uses-react rule for fbt, though. |
There is an easy solution/workaround for FBT users: set 'no-unused-vars': [
ERROR,
{
args: 'after-used',
ignoreRestSiblings: true,
varsIgnorePattern: '^fbt$', // <<< THIS
},
], @ljharb from the aforementioned solutions, which one do you think is the way to go?
|
The first option is the simplest, but would cause problems if that variable name was used in your codebase for something different. This risk might be acceptable inside a company's app. The second and third options would provide the most value for fbt users (not that i think there's many outside of facebook). By doing the third option, this repo would have to start taking a dev dependency on fbt and start using the jsx transform, in order to test it. The second option seems more "granular" to me. |
Thanks for the useful comments @ljharb. Do you think this issue is worth closing? There is an existing workaround and the number of affected users is probably quite low. Moreover, the BC break is because of |
This change updates `eslint-plugin-react` to the latest version 7.26.0 and workarounds `fbt` oddities discovered in the previous version (jsx-eslint/eslint-plugin-react#3080). I added 2 fixtures to make sure `no-unused-vars` rule works as expected with FBT. Additionally, it removes unused suppression comments that were also fixed in previous React plugin version (jsx-eslint/eslint-plugin-react#3063). Changelog: https://github.com/yannickcr/eslint-plugin-react/blob/eeb0144f14aa972f533e2ef0b094f6742d63c492/CHANGELOG.md#7260---20210920
This change updates `eslint-plugin-react` to the latest version 7.26.0 and workarounds `fbt` oddities discovered in the previous version (jsx-eslint/eslint-plugin-react#3080). I added 2 fixtures to make sure `no-unused-vars` rule works as expected with FBT. Additionally, it removes unused suppression comments that were also fixed in previous React plugin version (jsx-eslint/eslint-plugin-react#3063). Changelog: https://github.com/yannickcr/eslint-plugin-react/blob/eeb0144f14aa972f533e2ef0b094f6742d63c492/CHANGELOG.md#7260---20210920
I think until we get at least a second commenter on the repo ever that uses fbt, it’s probably best to close :-) |
This change updates `eslint-plugin-react` to the latest version 7.26.0 and workarounds `fbt` oddities discovered in the previous version (jsx-eslint/eslint-plugin-react#3080). I added 2 fixtures to make sure `no-unused-vars` rule works as expected with FBT. Additionally, it removes unused suppression comments that were also fixed in previous React plugin version (jsx-eslint/eslint-plugin-react#3063). Changelog: https://github.com/yannickcr/eslint-plugin-react/blob/eeb0144f14aa972f533e2ef0b094f6742d63c492/CHANGELOG.md#7260---20210920
This change updates `eslint-plugin-react` to the latest version 7.26.0 and workarounds `fbt` oddities discovered in the previous version (jsx-eslint/eslint-plugin-react#3080). I added 2 fixtures to make sure `no-unused-vars` rule works as expected with FBT. Additionally, it removes unused suppression comments that were also fixed in previous React plugin version (jsx-eslint/eslint-plugin-react#3063). Changelog: https://github.com/yannickcr/eslint-plugin-react/blob/eeb0144f14aa972f533e2ef0b094f6742d63c492/CHANGELOG.md#7260---20210920
This change updates `eslint-plugin-react` to the latest version 7.26.0 and workarounds `fbt` oddities discovered in the previous version (jsx-eslint/eslint-plugin-react#3080). I added 2 fixtures to make sure `no-unused-vars` rule works as expected with FBT. Additionally, it removes unused suppression comments that were also fixed in previous React plugin version (jsx-eslint/eslint-plugin-react#3063). Changelog: https://github.com/yannickcr/eslint-plugin-react/blob/eeb0144f14aa972f533e2ef0b094f6742d63c492/CHANGELOG.md#7260---20210920
This change updates `eslint-plugin-react` to the latest version 7.26.0 and workarounds `fbt` oddities discovered in the previous version (jsx-eslint/eslint-plugin-react#3080). I added 2 fixtures to make sure `no-unused-vars` rule works as expected with FBT. Additionally, it removes unused suppression comments that were also fixed in previous React plugin version (jsx-eslint/eslint-plugin-react#3063). Changelog: https://github.com/yannickcr/eslint-plugin-react/blob/eeb0144f14aa972f533e2ef0b094f6742d63c492/CHANGELOG.md#7260---20210920 adeira-source-id: 97d82ba734142c6ba2e1c37e3ab006477659b77c
This change updates `eslint-plugin-react` to the latest version 7.26.0 and workarounds `fbt` oddities discovered in the previous version (jsx-eslint/eslint-plugin-react#3080). I added 2 fixtures to make sure `no-unused-vars` rule works as expected with FBT. Additionally, it removes unused suppression comments that were also fixed in previous React plugin version (jsx-eslint/eslint-plugin-react#3063). Changelog: https://github.com/yannickcr/eslint-plugin-react/blob/eeb0144f14aa972f533e2ef0b094f6742d63c492/CHANGELOG.md#7260---20210920 adeira-source-id: 97d82ba734142c6ba2e1c37e3ab006477659b77c
Hey guys, it seems this bug happens with other types of tags too. Please see my example below. After updating to v7.26.0 I started to get the following
These elements are imported from import {
Helmet,
title,
link,
meta,
style,
} from 'react-native-web-ui-components/Helmet'; What do you think I should do? Use the same workaround as @mrtnzlml did? |
@carelulu-igor all lowercase tags in jsx are HTML tags. You can't import The above case is using fbt, a custom jsx transform. The way the rest of the ecosystem uses jsx, what I said above applies. In React Native, I would expect |
Hello! 👋 I think the problem I am going to describe is because of this PR: #3070 (cc @alanorozco).
Basically, after upgrading
eslint-plugin-react
from 7.25.1 to 7.25.2 (patch release), we started getting the following error in many files:I guess it's because FBT is a bit strange React citizen and the looks like this:
But unfortunately, after the aforementioned PR,
fbt
is no longer being taken into account andno-unused-vars
rule throws an error.Thanks for having a look! 😎 What would be the right way how to fix this issue?
The text was updated successfully, but these errors were encountered: