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

[Feature Request]: Add allowedElements option to jsx-no-literals #3808

Closed
1 task done
Pearce-Ropion opened this issue Aug 29, 2024 · 4 comments · Fixed by #3812
Closed
1 task done

[Feature Request]: Add allowedElements option to jsx-no-literals #3808

Pearce-Ropion opened this issue Aug 29, 2024 · 4 comments · Fixed by #3812

Comments

@Pearce-Ropion
Copy link
Contributor

What rule do you want to change?

jsx-no-literals

How do you think the change should be implemented?

I would like to see an option to add specific JSXElements to an allow list in jsx-no-literals. These elements would be allowed to have literals as children. I am unsure if this should also apply the other options that this rule provides such as allowing literals in props.

The new option could be named allowedElements and accept an array of allowed JSX intrinsic element names as well as named JSX element. If the named element is imported, it should use the imported name (so Button not MyButton in the case of import { Button as MyButton } from './Button';. If the import is a default import, it should match against the name given in the import (so MyButton in the case of import MyButton from './Button';.

I am unsure of the best way to handle namespaced elements as I could see it going either way. Maybe check each portion of the namespaced name as well as the whole thing.

Example

/* eslint react/jsx-no-literals: ["error", { "allowedElements": ["div"] }] */

<span>This should error</span>
<div>This should pass</div>

Additional Info

Currently, we use jsx-no-literals to prevent JSXText from appearing as the child of an element and instead require developers to pass their copy in via a translation function. This is similar to one of the examples in the jsx-no-literals docs. However, we use react-i18next which has a <Trans> component which will automatically pull all JSXText as valid translatable copy. We don't encourage our devs to use this method as it currently violates jsx-no-literals but would like to make this method less "hacky" when it is needed.

/* eslint react/jsx-no-literals: ["error", { "allowedElements": ["Trans"] }] */

<div>This should error</div>
<Trans>This should <a href="#">pass</a></Trans>

Participation

  • I am willing to submit a pull request to implement this change.
@ljharb
Copy link
Member

ljharb commented Aug 29, 2024

ah ok, so it's not about div/span, but about custom components specifically? That makes sense to me - at Airbnb we had a <T> component for the same use case.

@Pearce-Ropion
Copy link
Contributor Author

ah ok, so it's not about div/span, but about custom components specifically? That makes sense to me - at Airbnb we had a <T> component for the same use case.

Right, although I think it should still accept an intrinsic element if a user would like it to be configured that way.

How would you expect this option to interact with the other options this rule supports? Do you think it should basically constitute as a blanket "ignore anything with this element name", thus render all of the other options that affect the given element inert?

Or, should there be multiple versions of this option to allow enabling/disabling it for the other options? For example, the type could look something like this:

allowedElements?:
  | string[];
  | {
      elements: string[];
      noStrings?: boolean;
      allowedStrings?: boolean;
      ignoreProps?: boolean;
      noAttributeStrings?: boolean;
    };

Or you might be able to specify the list per option like this:

allowedElements?:
  | string[];
  | {
      noStrings?: string[];
      allowedStrings?: string[];
      ignoreProps?: string[];
      noAttributeStrings?: string[];
    }

@ljharb
Copy link
Member

ljharb commented Aug 30, 2024

I'd prefer not to allow html elements to start - if someone wants that, they'd need to provide a compelling justification.

hm, good question. what about a config option elementOverrides, that takes an object, whose keys are a custom element name (if it doesn't start with a capital letter it's invalid), and the value is the root config object minus elementOverrides?

@Pearce-Ropion
Copy link
Contributor Author

Ooh, I like it!

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

Successfully merging a pull request may close this issue.

2 participants