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

Proposal: no bare strings in template #491

Closed
patric-eberle opened this issue Jun 6, 2018 · 11 comments · Fixed by #1185
Closed

Proposal: no bare strings in template #491

patric-eberle opened this issue Jun 6, 2018 · 11 comments · Fixed by #1185

Comments

@patric-eberle
Copy link

Please describe what the rule should do:
Projects which use localization (e.g. vue-i18n) should not allow the use of bare strings (hardcoded strings) in templates. They should force the use the mustache syntax or directives.

This proposal is based on the Ember template linter, which includes such a rule already: https://github.com/ember-template-lint/ember-template-lint/blob/master/docs/rule/no-bare-strings.md

What category of rule is this? (place an "X" next to just one item)

[x] Enforces code style
[ ] Warns about a potential error
[ ] Suggests an alternate way of doing something
[ ] Other (please specify:)

Provide 2-3 code examples that this rule will warn about:
Invalid template content

<h1>Foo baa</h1>

Valid template content

<h1>{{ $t('myTranslationKey') }}</h1>
<h1>{{ aComputedValue }}</h1>
<h1 v-t="'foo.bar'"></h1>
<h1><span>{{ aComputedValue }}</span></h1>

Why should this rule be included?
It allows to test templates for hardcoded text which most likely will lead to issues in localized projects.

@patric-eberle patric-eberle changed the title Proposale: no bare strings in template Proposal: no bare strings in template Jun 6, 2018
@michalsnik
Copy link
Member

I like this idea! I always used this rule in Ember projects 👍

@chrisvfritz
Copy link
Contributor

chrisvfritz commented Jul 31, 2018

This is indeed a good rule for localization! Since it wouldn't apply to many projects though, I think it would be uncategorized. One question though: should the following be valid?

<h1 v-t="'foo.bar'"></h1>

Or should it instead be:

<h1 v-t="foo.bar"></h1>

?

@patric-eberle
Copy link
Author

@chrisvfritz I would recommend to not check this with eslint because both usages are valid. I think for what you want to do the first one would be correct. See https://kazupon.github.io/vue-i18n/api/#directives - Still the second example with no single quotes would also be valid if you want to bind an instance property.

@chrisvfritz
Copy link
Contributor

Ah, I see. Then we may want to disable string literals for directives, with a whitelist of some directives, like v-t and the user could add their own as well. Otherwise, we open up a big hole with v-text, v-html, and directives defined by other libraries, like v-tooltip. For example:

<p v-text="'hello'"/>
<button v-tooltip="'Hard-coded button tooltip content'">

We probably don't want those to be considered valid, right?

@patric-eberle
Copy link
Author

Ah, I see what you mean. I think your recommondation makes sense.

@michalsnik
Copy link
Member

Also depending on element the white-list might look a bit different.

Let's take <input> tag as example.

<input type="text" :class="classObj" value="1" style="width: 100%" disabled="disabled">

Above HTML probably shouldn't trigger any error. But setting value attribute on other component might.

The most straightforward cases are at this moment:

<div>plain text</div>
<div>{{ 'another plain text' }}</div>

I'll try to come up with an idea how to handle the rest of the cases to not introduce unnecessary false-positives.

@michalsnik
Copy link
Member

I gave it a second thought and I like how it is solved in ember-template-lint. I used their rule in all of my commercial Ember projects and it actually worked really well. I don't think we should go crazy here, and perhaps similar approach would suffice.

The following cases might be considered warnings then:

  1. Plain text, excluding signs like: (),.&+-=*/#%!?:[]{}.
<div>text</div>
  1. Global attributes on only-HTML elements:
    By default these attributes: title, aria-label, aria-placeholder, aria-roledescription, aria-valuetext
<div
  title="text"
  aria-label="label"
  aria-placeholder="placeholder"
  aria-roledescription="desc"
  aria-valuetext="text"
/>
  1. Tag specific attributes:
    By default: { img: ['alt'], input: ['placeholder'] }
<img alt="text">
<input placeholder="text">
  1. Certain directives
    By default: ['v-text', 'v-html']
<div v-text="'lorem ipsum'" />
<div v-html="'lorem ipsum'" />

Users could add more attributes or directives to make this rule more rigid if they want.

Also I'm not sure if we should report a warning when user explicitly returns a string in an expression:

{{ 'text' }}

This looks like a conscious decision to me, similar to bindings:

<a :title="'lorem ipsum" />

What do you think @chrisvfritz @patric-eberle ?

@patric-eberle
Copy link
Author

@michalsnik I think keeping it simple should be ok. I think the rule makes most sense for content and static text attributes. I also think it‘s not necessary to test bindings and expressions.

@chrisvfritz
Copy link
Contributor

Sounds good to me. 🙂

@lbennett-stacki
Copy link
Contributor

Just an FYI, I've got a branch for this but I need some time to test it on a large codebase to discover edge cases that require additional logic and unit tests. It seems far too simple as it is. 😂

@nilesh9836
Copy link

can anybody help me with how to use this rule in my package.json

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

Successfully merging a pull request may close this issue.

5 participants