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

[New] `jsx-no-literals`: add `noAttributeStrings` option #2782

Merged
merged 1 commit into from Sep 16, 2020

Conversation

@TaLeaMonet
Copy link
Contributor

@TaLeaMonet TaLeaMonet commented Sep 2, 2020

Closes #2496. Fixes #2486.

Co-authored-by: TaLea Carpenter <taleacarpenter@gmail.com>
Co-authored-by: tanmoyopenroot <tanmoy.openroot@gmail.com>

Fixes ##2486
}
},

This comment has been minimized.

@karolina-benitez

karolina-benitez Sep 2, 2020
Contributor

Remove unnecessary new line

This comment has been minimized.

@ljharb

ljharb Sep 12, 2020
Collaborator

+1, altho if it helps readability it'd be fine

This comment has been minimized.

@TaLeaMonet

TaLeaMonet Sep 15, 2020
Author Contributor

New line removed.

@ljharb ljharb changed the title [Fix] support jsx-no-literals on specific attributes [New] support jsx-no-literals on specific attributes Sep 5, 2020

* `noStrings` (default: `false`) - Enforces no string literals used as children, wrapped or unwrapped.
* `allowedStrings` - An array of unique string values that would otherwise warn, but will be ignored.
* `ignoreProps` (default: `false`) - When `true` the rule ignores literals used in props, wrapped or unwrapped.
* `noAttributeStrings` (default: `false` ) - Enforces no string literals used in attributes when set to `true`.

This comment has been minimized.

@ljharb

ljharb Sep 5, 2020
Collaborator

Suggested change
* `noAttributeStrings` (default: `false` ) - Enforces no string literals used in attributes when set to `true`.
* `noAttributeStrings` (default: `false`) - Enforces no string literals used in attributes when set to `true`.
additionalProperties: false
}]
},

create(context) {
const defaults = {noStrings: false, allowedStrings: [], ignoreProps: false};
const defaults = {
noStrings: false, allowedStrings: [], ignoreProps: false, noAttributeStrings: false

This comment has been minimized.

@ljharb

ljharb Sep 5, 2020
Collaborator

Suggested change
noStrings: false, allowedStrings: [], ignoreProps: false, noAttributeStrings: false
noStrings: false,
allowedStrings: [],
ignoreProps: false,
noAttributeStrings: false

when the entire object doesn't fit on one line, each property should be on its own line.

function defaultMessage() {
switch (true) {
case config.noAttributeStrings:
Comment on lines 60 to 62

This comment has been minimized.

@ljharb

ljharb Sep 5, 2020
Collaborator

please use an if with early returns here instead of a switch.


context.report({
node,
message: `${errorMessage}: ${context.getSourceCode().getText(node).trim()}`
message: `${errorMessage}: "${context.getSourceCode().getText(node).trim()}"`

This comment has been minimized.

@ljharb

ljharb Sep 5, 2020
Collaborator

the curly quotes here are quite intentional; straight quotes are never appropriate in prose. please revert this line.

Copy link
Collaborator

@ljharb ljharb left a comment

LGTM pending comments

@@ -26,16 +26,17 @@ var Hello = <div>

## Rule Options

There are two options:
There are three options:

This comment has been minimized.

@ljharb

ljharb Sep 12, 2020
Collaborator

Probably a good idea to remove the count, to avoid having to update this in the future (also you're changing it to "three" but i think there's 4 now?):

Suggested change
There are three options:
The supported options are:

This comment has been minimized.

@TaLeaMonet

TaLeaMonet Sep 15, 2020
Author Contributor

Good catch, the wording has been changed.

}
},

This comment has been minimized.

@ljharb

ljharb Sep 12, 2020
Collaborator

+1, altho if it helps readability it'd be fine

&& parent.type !== 'JSXAttribute';

function isParentNodeStandard() {
if (!/^[\s]+$/.test(node.value) && typeof node.value === 'string' && parent.type.indexOf('JSX') !== -1) {

This comment has been minimized.

@ljharb

ljharb Sep 12, 2020
Collaborator

Suggested change
if (!/^[\s]+$/.test(node.value) && typeof node.value === 'string' && parent.type.indexOf('JSX') !== -1) {
if (!/^[\s]+$/.test(node.value) && typeof node.value === 'string' && parent.type.includes('JSX')) {
@ljharb ljharb added the enhancement label Sep 12, 2020
@ljharb ljharb changed the title [New] support jsx-no-literals on specific attributes [New] `jsx-no-literals`: add `noAttributeStrings` option Sep 16, 2020
@ljharb ljharb force-pushed the TaLeaMonet:issue2486 branch from 1929be2 to 153eac8 Sep 16, 2020
@ljharb
ljharb approved these changes Sep 16, 2020
@ljharb ljharb merged commit 153eac8 into yannickcr:master Sep 16, 2020
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.005%) to 97.495%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.