-
-
Notifications
You must be signed in to change notification settings - Fork 70
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
feat: support localization for rule messages #128
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting this together. As I noted inline, part of the RFC process is to figure out how this might be implemented. What you've provided is a high-level description, but in order to really evaluate this, we need an idea about the implementation details.
Explain the design with enough detail that someone familiar with ESLint | ||
can implement it by reading this document. Please get into specifics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note, you'll need to provide a description of how this would be implemented for the RFC to move forward.
messages: { | ||
someMessageId: { | ||
en: "Default message in English", | ||
es: "Mensaje predeterminado en español" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a fan of overloading message IDs like this. I'd rather introduce another key, messageTranslations
at the same level as messages
, if we want this to be included in the rule file itself.
rules: [/* ... */], | ||
messages: { | ||
"plugin/ruleId": { | ||
someMessageId: "Mensaje predeterminado en español" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you intending for people to include their own translations in config files? Or pulling in from somewhere else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also pointing out that message ids aren't generally public api of a plugin. This approach would change that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you intending for people to include their own translations in config files? Or pulling in from somewhere else?
I think we should provide three ways to include translations:
- Through the plugin module itself.
- Through another plugin specifically designed to support translations, such as
@types/module
. - Through a configuration file, allowing users to quickly add translations, such as writing a custom
@types/module
definition specific to their project.
So it is part of the way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also pointing out that message ids aren't generally public api of a plugin. This approach would change that.
This seems to be a challenge, but I'd like to avoid making a breaking change for this if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also pointing out that message ids aren't generally public api of a plugin. This approach would change that.
Yeah, I think for this to work, we need to just say that message IDs are part of the public API. Unless we want to force every rule to maintain its own translations, I think that's our only option.
|
||
```js | ||
module.exports = { | ||
locale: "es", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does locale
do here?
Does it only apply to files matching files
/ignores
? (This is how config objects work in the config array, there's no such thing as a global setting.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just define the expected language the user wants to see in the lint error messages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I think I was unclear. If you have a config like this:
{
files: ["**/*.js"],
locale: "es"
}
That config is only applied when the file being linted matches **/*.js
. Is that was you intended?
implementing this RFC as possible. | ||
--> | ||
|
||
1. Increased Maintenance Burden: Adding support for localization will increase the complexity of the ESLint codebase. Maintaining translations and ensuring they are up-to-date with rule changes will require additional effort from both the core team and contributors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also increases the download size.
--> | ||
|
||
1. Increased Maintenance Burden: Adding support for localization will increase the complexity of the ESLint codebase. Maintaining translations and ensuring they are up-to-date with rule changes will require additional effort from both the core team and contributors. | ||
2. It's better to use a module to handle the localization such as typescript using `@types/some-module` but it's not possible to use a module to handle the localization in ESLint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What part isn't possible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to scan the entire node_modules directory to locate the @i18n/module
for each eslint-plugin-module
and implement a mechanism to load the @i18n/module
. Does this approach seem reasonable?"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I can research it further to see if this approach is worth pursuing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see. Yes, we don't want ESLint to scan the node_modules
directory (if ESLint isn't running in Node.js, that directory might not exist). We'd need people to manually import the module into their config file.
> [TypeScript compiler options](https://www.typescriptlang.org/docs/handbook/compiler-options.html#compiler-options) | ||
> https://github.com/eslint/eslint/issues/9870#issuecomment-359228967 | ||
|
||
why not? because almost all the messages are provided by third party plugins, so it's not possible to handle the localization in the core. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't we consider an approach where translations are provided by third party plugins?
to existing users? | ||
--> | ||
|
||
This change should not affect existing ESLint users who do not use the localization feature. Users who do not provide translations will continue to see the default English messages. To minimize disruption, ESLint should provide clear documentation on how to create and use localization files, as well as guidelines for contributing translations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Backwards compatibility analysis should consider plugin compatibility as well as end user / config compatibility. What's the implication to plugin authors who haven't set up the locale-based message ids in their rules?
module.exports = { | ||
locale: "es", | ||
rules: [/* ... */], | ||
messages: { | ||
"plugin/ruleId": { | ||
someMessageId: "Mensaje predeterminado en español" | ||
} | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you clarify this structure? Does this mean that provided messages
apply to locale: "es"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the locale means user want to use 'es' language, and the messages means some plugins haven't support a language message so that user can override it by theirself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gweesin please add that description into the RFC.
Just a heads up for some prior art that should be linked to in the RFC: |
@gweesin Just a reminder that there are some suggestions for you on this RFC. |
Summary
This RFC proposes adding support for localization of rule messages in ESLint. The goal is to allow users to provide translations for rule messages, enabling ESLint to display messages in different languages based on user preferences. This feature aims to improve the accessibility and usability of ESLint for non-English speaking users.
Related Issues