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.

Sign up for GitHub

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

feat(no-unlocalized-strings): remove default configuration #78

Merged
merged 5 commits into from
Nov 14, 2024
Merged

feat(no-unlocalized-strings): remove default configuration #78

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f70111e
feat(no-unlocalized-strings): remove default configuration
timofei-iatsenko Nov 4, 2024
cbc587c
feat(no-unlocalized-strings): ignore non letter strings by default
timofei-iatsenko Nov 12, 2024
cf1397a
feat(no-unlocalized-strings): allow numbers in uppercase regex
timofei-iatsenko Nov 12, 2024
0a2409e
feat(no-unlocalized-strings): merge ignoreAttribute, ignoreProperty a…
timofei-iatsenko Nov 12, 2024
0bfae9b
feat(no-unlocalized-strings): remove strictAttribute option
timofei-iatsenko Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ yarn add eslint-plugin-lingui --dev

### Recommended Setup

To enable all of the recommended rules for our plugin, add the following config:
To enable all the recommended rules for our plugin, add the following config:

```js
import pluginLingui from 'eslint-plugin-lingui'
Expand All @@ -47,6 +47,8 @@ export default [
]
```

We also recommend enabling the [no-unlocalized-strings](docs/rules/no-unlocalized-strings.md) rule. It’s not enabled by default because it needs to be set up specifically for your project. Please check the rule's documentation for example configurations.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see why you want to go in this direction, but personally I had a lot of help from the defaults, and might have been hesitant to even try to use the rule if faced with a custom setup.

UPPERCASE_LETTERS being ignored was fine imo. My 10/10 experience would be what we had, including the ability to override such specifics if desired (unsure how feasible that is).

Either way, I'm a bit wary here, thinking that this PR may be a move away from a 9/10 out of the box experience which frankly is pretty good.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is a could not come up with a good way to override defaults. So we have two situations:

  1. When user ok with provided defaults and want to add more specificity for the project
  2. User is not ok with defaults and want to revert some of the buil-in rules (that already happened few times, saying from issues)

First case is already supported, we can extend the defaults.

The second case is a bit trickier, you need to give a way for user to choose what is going to happen, override or extend? This either brings more complicated structure of every option or double of options (ignorePoperty, ignorePropertyOverride). I don't like both of variants, actually.

That's why I decided to give a blank list, so there is nothing to override and it's pretty configurable. Also, it gives much more clarity about what exactly is ignored

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solid reasoning.

Your example is thorough and should be easy enough to copy paste and run with. I might have been further encouraged if this language

You can use the following config as a starting point and then adjust it for your project

was slightly more assertive, i.e. officially recommending it, or mentioning it's history as the de facto-rule set before opening up for configuration.


### Custom setup

Alternatively, you can load the plugin and configure only the rules you want to use:
Expand Down
103 changes: 88 additions & 15 deletions docs/rules/no-unlocalized-strings.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,93 @@ Ensures that all string literals, templates, and JSX text are wrapped using `<Tr
> [!IMPORTANT]
> This rule may require TypeScript type information. Enable this feature by setting `{ useTsTypes: true }`.

This rule is designed to **match all** JSXText, StringLiterals, and TmplLiterals, and then exclude some of them based on attributes, property names, variable names, and so on.

The rule doesn’t come with built-in ignore settings because each project is unique and needs different configurations. You can use the following config as a starting point and then adjust it for your project:

<!-- prettier-ignore -->
```json5
{
"no-unlocalized-strings": [
"error",
{
"useTsTypes": true,
"ignore": [
// Ignore strings that do not contain words
"^[^A-Za-z]+$",
timofei-iatsenko marked this conversation as resolved.
Show resolved Hide resolved
// Ignore strings that don’t start with an uppercase letter
// or don't contain two words separated by whitespace
"^(?![A-Z].*|\\w+\\s\\w+).+$",
// Ignore UPPERCASE literals
// Example: const test = "FOO"
"^[A-Z_-]+$"
],
"ignoreAttribute": [
// Ignore attributes matching className (case-insensitive)
{ "regex": { "pattern": "className", "flags": "i" } },
"styleName",
"src",
"srcSet",
"type",
"id",
"width",
"height"
],
"ignoreProperty": [
// Ignore properties matching className (case-insensitive)
{ "regex": { "pattern": "className", "flags": "i" } },
"styleName",
"type",
"id",
"width",
"height",
"displayName",
"Authorization",
// Ignore UPPERCASE properties
// Example: test.FOO = "ola!"
{ "regex": { "pattern": "^[A-Z_-]+$" } }
],
"ignoreFunction": [
"cva",
"cn",
"track",
"Error",
"console.*",
"*headers.set",
"*.addEventListener",
"*.removeEventListener",
"*.postMessage",
"*.getElementById",
"*.dispatch",
"*.commit",
"*.includes",
"*.indexOf",
"*.endsWith",
"*.startsWith",
"require"
],
"ignoreVariable": [
// Ignore literals assigned to variables with UPPERCASE names
// Example: const FOO = "Ola!"
{ "regex": { "pattern": "^[A-Z_-]+$" } }
],
"ignoreMethodsOnType": [
// Ignore specified methods on Map and Set types
"Map.get",
"Map.has",
"Set.has"
]
}
]
}
```

## Options

### `useTsTypes`

Enables the rule to use TypeScript type information. Requires [typed linting](https://typescript-eslint.io/getting-started/typed-linting/) to be configured.

This option automatically excludes built-in methods such as `Map` and `Set`, and cases where string literals are used as TypeScript constants, e.g.:

```ts
const a: 'abc' = 'abc'
```

### `ignore`

Specifies patterns for string literals to ignore. Strings matching any of the provided regular expressions will not trigger the rule.
Expand Down Expand Up @@ -77,7 +152,7 @@ foo[getName()].set('Hello')

### `ignoreAttribute`

Specifies JSX attributes that should be ignored. By default, the attributes `className`, `styleName`, `type`, `id`, `width`, and `height` are ignored.
Specifies JSX attributes that should be ignored.

Example for `{ "ignoreAttribute": ["style"] }`:

Expand Down Expand Up @@ -158,16 +233,16 @@ const element = <div description="IMAGE" />

### `ignoreProperty`

Specifies object property names whose values should be ignored. By default, UPPERCASED properties and `className`, `styleName`, `type`, `id`, `width`, `height`, and `displayName` are ignored.
Specifies object property names whose values should be ignored.

Example for `{ "ignoreProperty": ["myProperty"] }`:
Example for `{ "ignoreProperty": ["displayName"] }`:

```jsx
const obj = { myProperty: 'Ignored value' }
obj.myProperty = 'Ignored value'
const obj = { displayName: 'Ignored value' }
obj.displayName = 'Ignored value'

class MyClass {
myProperty = 'Ignored value'
displayName = 'Ignored value'
}
```

Expand Down Expand Up @@ -208,7 +283,7 @@ class MyClass {

### `ignoreVariable`

Specifies variable name whose values should be ignored. By default, UPPERCASED variables are ignored.
Specifies variable name whose values should be ignored.

Example for `{ "ignoreVariable": ["myVariable"] }`:

Expand Down Expand Up @@ -264,5 +339,3 @@ interface Foo {
const foo: Foo
foo.get('Some string')
```

The following methods are ignored by default: `Map.get`, `Map.has`, `Set.has`.
38 changes: 1 addition & 37 deletions src/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ export const LinguiCallExpressionMessageQuery =
*/
export const LinguiTransQuery = 'JSXElement[openingElement.name.name=Trans]'

export const UpperCaseRegexp = /^[A-Z_-]+$/

export function isNativeDOMTag(str: string) {
return DOM_TAGS.includes(str)
}
Expand All @@ -39,27 +37,13 @@ export function isSvgTag(str: string) {
return SVG_TAGS.includes(str)
}

export function containAllAttributes(attributeNames: string[]) {
const attrs = ['value', 'other']
return attrs.every((attr) => attributeNames.includes(attr))
}

export function isLinguiTags(str: string, attributeNames: string[]) {
if (str === 'Trans') {
return true
}
return ['Plural', 'Select'].includes(str) && containAllAttributes(attributeNames)
}

const blacklistAttrs = ['placeholder', 'alt', 'aria-label', 'value']
export function isAllowedDOMAttr(tag: string, attr: string, attributeNames: string[]) {
if (isSvgTag(tag)) return true
if (isNativeDOMTag(tag)) {
return !blacklistAttrs.includes(attr)
}
if (isLinguiTags(tag, attributeNames)) {
return true
}

return false
}

Expand Down Expand Up @@ -87,26 +71,6 @@ export const getText = (
return node.value.toString().trim()
}

export function hasAncestorWithName(
node: TSESTree.JSXElement | TSESTree.TemplateLiteral | TSESTree.Literal | TSESTree.JSXText,
name: string,
) {
let p: TSESTree.Node = node.parent
while (p) {
switch (p.type) {
case TSESTree.AST_NODE_TYPES.JSXElement:
const identifierName = getIdentifierName(p?.openingElement?.name)
if (identifierName === name) {
return true
}
default:
}

p = p.parent
}
return false
}

export function getIdentifierName(jsxTagNameExpression: TSESTree.JSXTagNameExpression) {
switch (jsxTagNameExpression.type) {
case TSESTree.AST_NODE_TYPES.JSXIdentifier:
Expand Down
Loading