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

"mode": 'jsx-only' forbids any string literal inside interpolation blocks #109

Open
lgenzelis opened this issue May 30, 2023 · 6 comments
Open

Comments

@lgenzelis
Copy link
Contributor

lgenzelis commented May 30, 2023

I'm using "eslint-plugin-i18next": "6.0.1".

My use case: I want to forbid string literals in children and aria-labels. So, this is my config:

'i18next/no-literal-string': [
  'error',
  {
    "mode": 'jsx-only',
    'jsx-attributes': { include: ['aria-label'] },
  },
],

This works fine, except that it detects any string literal inside an interpolation block as an error. For example:

const list = ['a', 'b', 'c'];

function Foo() {
  console.log('rending foo'); // ----> This is fine
  return (
    <div>
      {list.map((item) => {
        console.log('debugging', item); // -------> An error is reported in the word 'debugging'
        return <div key={item}>{item}</div>;
      })}
    </div>
  );
}
@edvardchen
Copy link
Owner

jsx-only mode did check everything surrounded by JSX markup.

My initial thought is that people might make mistakes easily like

<>hello world</>  // error
// vs
<>{"hello world"}</> // but this's fine?

@lgenzelis
Copy link
Contributor Author

Ok, I get it. In your example, both should be errors, indeed. The difference, I guess, is that in my example the "wrongly" detected error is inside a callback (for .map()). I don't know any details about the implementation, but maybe you could check if a callback is being declared, and in that scenario, "forget" about any JSX ancestor?

As a side question, is it ok that I need to specify "mode": 'jsx-only', for my use case? Or is that a bug? I mean, if I just do

'i18next/no-literal-string': [
  'error',
  {
    'jsx-attributes': { include: ['aria-label'] },
  },
],

then aria-labels are not checked at all.

@edvardchen
Copy link
Owner

The default mode is jsx-text-only which can be found here.

but maybe you could check if a callback is being declared, and in that scenario, "forget" about any JSX ancestor?

Actually the behavior of mode jsx-text-only is very close to this. But It only checks literals JSX text like <>hello world</>

Since the existing mode all jsx-only jsx-text-only couldn't satisfy your need, I think it needs a new mode which checks only literal JSX text and JSX attributes

@lgenzelis
Copy link
Contributor Author

lgenzelis commented Jun 6, 2023

Do you think that a new mode is really necessary? I mean, in my original example, do you think there are cases using jsx-only where that behavior is desired? In my opinion it looks more like an improvement that could be done to the jsx-only mode (I mean, ignoring non-jsx related strings inside CallExpressionss 🤔 )

@lgenzelis
Copy link
Contributor Author

I gave it a try @edvardchen ! 😁 Please let me know what you think #110 🙏

@edvardchen
Copy link
Owner

It will be a breaking change since we change the behavior of mode jsx-only

Your proposal would make function calls <>{display("hello")} </> <>[0].map(()=>"hello")</> valid but <>{["name"][0]}</> invalid which is a bit confusing. Hard to understand why. It would feel like we had left something unfinished.

Instead, I prefer to check or not to check these dynamic part at all. We can make a new mode named jsx-static-only which checks only JSX text and attributes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants