[New]: `jsx-no-literals`: add `ignoreProps` option to ignore props validation #2146

iiison · 2019-01-29T07:43:33Z

Update Tests Accordingly
Update Rule Readme file
Rule shouldn't allow strings in empty tags

…lidation Co-authored-by: Bharat Soni <i.bharat.soni@gmail.com> Co-authored-by: Serg Hospodarets <shospodarets@gmail.com>

tests/lib/rules/jsx-no-literals.js

iiison · 2019-02-07T06:55:48Z

@ljharb added new option to validate props. I've set it to true by default so that behaviour stays the same, user can set the value to false in order to remove props validation.

One more thing, rule passes if prop is a strings literal(<div className="abc" />), but if the prop value is a template literal, then rule will throw error. This behaviour was there from starting, LMK if you want to change this.

iiison · 2019-02-07T06:58:22Z

@ljharb the build is breaking from starting, it works fine in my local, is it because I am using ES6 syntax? Could you please help me if you are aware of the error..

ljharb · 2019-02-07T07:00:51Z

docs/rules/jsx-no-literals.md


 * `noStrings` - Enforces no string literals used as children, wrapped or unwrapped.
+* `validateProps` - Enforces no template literals used as props.


why only template? i'd expect any literals to be detected by this option

I was trying to mimic the same behaviour as before, will detect string literals too.

docs/rules/jsx-no-literals.md

tests/lib/rules/jsx-no-literals.js

iiison · 2019-02-07T07:56:52Z

@ljharb I see you removed your comment to detect and disallow string literals in props, shall I add that feature or not..

ljharb · 2019-02-07T07:57:44Z

@iiison i marked it resolved because you indicated you'd incorporate it. There's no reason for a rule called "literals" to differentiate between string and template.

iiison · 2019-03-27T02:01:03Z

@ljharb my laptop was broken for some time, can you please provide the context of the last comment that you resolved.. I will fix it as soon as I get a little context of it.

ljharb · 2019-03-27T04:04:48Z

@iiison see #2146 (comment), i've unmarked it as resolved. basically, we want template and string literals checked.

iiison · 2019-04-03T05:38:58Z

@ljharb if we don't allow strings in props, then you need to declare variables for all props like type attribute in input tag element and for a lot of other places too. I think this will complicate things. Let me know your thoughts.

ljharb · 2019-04-06T06:37:19Z

@iiison sure, that's why you might not want to enable the option. There's no difference between foo="bar" and foo={"bar"} and foo={`bar`}.

iiison · 2019-04-22T06:15:48Z

@ljharb updated the PR. Added requested changes. But build is failing. The code is failing in few versions of Node(not sure why), can you please help me with that.

ljharb · 2019-04-22T21:52:50Z

lib/rules/jsx-no-literals.js

-    const sourceCode = context.getSourceCode();
+  create: function({
+    report,
+    options: [configs = {}],


does this syntax work in node 4?

ljharb · 2019-04-22T21:54:07Z

lib/rules/jsx-no-literals.js

-  create: function(context) {
-    const isNoStrings = context.options[0] ? context.options[0].noStrings : false;
-    const sourceCode = context.getSourceCode();
+  create: function({


let's try going back to the non-destructuring form.

ljharb · 2019-04-22T21:54:28Z

lib/rules/jsx-no-literals.js

+    } = configs;
+    const isNoStrings = noStrings;
+    const shouldValidateProps = validateProps;
+    const sourceCode = getSourceCode();


in particular, it seems that eslint 3 relies on the receiver (the this value) of getSourceCode being context.

iiison · 2019-04-25T06:10:57Z

fixed all errors!

iiison · 2019-04-26T03:15:38Z

@ljharb Let me know if you want to add more changes to the PR.

ljharb · 2019-04-26T06:32:43Z

tests/lib/rules/jsx-no-literals.js

+        {message: stringsMessage('\'bar\'')}
+      ]
+    }, {
+      code: '<Foo event={() => {}} />',


this rule is about string literals. why would a function value ever trigger an error?

I suppose I went a little too far on this. Will remove the logic.

ljharb · 2019-04-28T07:26:14Z

docs/rules/jsx-no-literals.md

+```
+
+```jsx
+var Hello = <div event={()=>{}} />;


should this be removed from the readme?

iiison · 2019-04-29T02:46:38Z

@ljharb I'm not sure why, but AppVeyor is failing out of nowhere. I just deleted 2 lines from the readme file, not sure why it is breaking.

ljharb · 2019-04-29T04:27:13Z

I'm not sure either, looks like a jest error - either way, could you try rebasing on latest master instead of merging?

iiison · 2019-04-29T05:03:01Z

The build was failing when I didn't merge the master in the branch. but will try the rebase as well.

iiison · 2019-04-29T07:32:04Z

rebased with the master, still, the build is failing with the same error.

iiison · 2019-04-29T07:40:46Z

@ljharb build is also failing for the master

ljharb · 2019-04-30T17:54:33Z

Blocked by #2253.

iiison · 2019-05-03T04:30:50Z

@ljharb as #2253 is closed, can we merge this branch...

mauricecruz · 2019-10-22T15:23:18Z

@iiison any interest in moving this forward? Happy to take a look if you're no longer vested in this fix.

iiison · 2019-10-30T11:28:16Z

@mauricecruz Please go ahead. I won't be able to work on this for a while.

dbassett17 · 2019-11-24T20:24:52Z

@mauricecruz @ljharb @iiison Any progress on this or is it still blocked? If nobody else has the time I can probably find some time to finish this up soon.

ljharb · 2019-11-28T08:00:02Z

tests/lib/rules/jsx-no-literals.js

      errors: [{message: stringsMessage('`Test`')}]
    }, {
      code: '<Foo bar={`${baz}`} />',
-      options: [{noStrings: true}],
+      options: [{noStrings: true, validateProps: true}],


These test changes show that the option (that doesn't exist yet) seems to already be true by default prior to this PR. In that case, this option should probably be more like ignoreProps: true which defaults to false.

@ljharb just checking... Is this the only pending requested change?

@Gasparila at the moment, yes - i'll have to give it a rereview after it's next updated

Just found this great react eslint rule to use in a big product and faced the same problem that it goes through the props, which was a bit surprising, but ok.
In short, the current rule behavior is to check props, facilitating it to be disabled by default may break some existing users functionalities, who may rely on this.
+1 , is should be either changed to ````validateProps(true` default)``` in the current naming, or switched to `ignoreProps`

ljharb · 2019-12-13T00:07:31Z

ping @iiison, are you still interested in completing this PR?

SalimBensiali · 2020-02-24T01:31:34Z

@ljharb @iiison, any update on this? Are there any requested changes still pending? Happy to chip in.

ljharb · 2020-02-25T06:24:29Z

@SalimBensiali if you want to post a link to your branch with the needed changes (do NOT post a new PR, please) then i'm happy to pull it in.

shospodarets · 2020-03-08T03:53:24Z

Hey @ljharb @SalimBensiali @iiison
I created a PR to switch to ignoreProps: true and adjusted the code, docs and tests to pass: iiison#1
It targets the current branch, can you please review when you have time, and let me know if you'd need me to merge it, or you'd prefer other way of having this arrived in the main repo

ljharb

Thanks @iiison and @malyw!

[New]: jsx-no-literals: add ignoreProps option to ignore props va…

fa863ce

…lidation Co-authored-by: Bharat Soni <i.bharat.soni@gmail.com> Co-authored-by: Serg Hospodarets <shospodarets@gmail.com>

iiison mentioned this pull request Jan 29, 2019

Getting wrong error "Strings not allowed in JSX files" in prop #2142

Closed

ljharb reviewed Jan 29, 2019

View reviewed changes

tests/lib/rules/jsx-no-literals.js Show resolved Hide resolved

ljharb requested changes Feb 7, 2019

View reviewed changes

ljharb mentioned this pull request Mar 28, 2019

react/jsx-no-literals: some strings are slipping through #2215

Open

ljharb requested changes Apr 22, 2019

View reviewed changes

ljharb reviewed Apr 26, 2019

View reviewed changes

ljharb reviewed Apr 28, 2019

View reviewed changes

docs/rules/jsx-no-literals.md Outdated

```

```jsx

var Hello = <div event={()=>{}} />;

Copy link

Member

ljharb Apr 28, 2019

Choose a reason for hiding this comment

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

should this be removed from the readme?

iiison force-pushed the string-litral-fix branch 2 times, most recently from 5ac178c to 29d70e6 Compare April 29, 2019 07:24

ljharb force-pushed the string-litral-fix branch from e76c282 to 8a1eb1e Compare April 30, 2019 07:15

ljharb approved these changes Apr 30, 2019

View reviewed changes

ljharb changed the title ~~FIX: Don't validate props in jsx-no-literals rule~~ [New]: jsx-no-literals: add validateProps option to ignore props validation Nov 28, 2019

ljharb force-pushed the string-litral-fix branch 2 times, most recently from 448f86c to cf2210c Compare November 28, 2019 07:52

ljharb reviewed Nov 28, 2019

View reviewed changes

ljharb force-pushed the string-litral-fix branch from cf2210c to 6dd14b9 Compare November 28, 2019 08:05

shospodarets mentioned this pull request Mar 8, 2020

[New]: jsx-no-literals: add validateProps option to ignore props … iiison/eslint-plugin-react#1

Merged

ljharb force-pushed the string-litral-fix branch 2 times, most recently from 122721f to 13e649e Compare March 11, 2020 00:10

ljharb changed the title ~~[New]: jsx-no-literals: add validateProps option to ignore props validation~~ [New]: jsx-no-literals: add ignoreProps option to ignore props validation Mar 11, 2020

ljharb added the enhancement label Mar 11, 2020

ljharb force-pushed the string-litral-fix branch from 13e649e to fa863ce Compare March 11, 2020 00:11

ljharb approved these changes Mar 11, 2020

View reviewed changes

ljharb merged commit fa863ce into jsx-eslint:master Mar 11, 2020

This was referenced Mar 15, 2021

Bump eslint-plugin-react from 7.14.3 to 7.22.0 statisticsnorway/design-system#164

Closed

Bump eslint-plugin-react from 7.19.0 to 7.22.0 newerton/gostack-template-fundamentos-reactjs#25

Merged


		* `noStrings` - Enforces no string literals used as children, wrapped or unwrapped.
		* `validateProps` - Enforces no template literals used as props.

[New]: jsx-no-literals: add ignoreProps option to ignore props validation #2146

[New]: jsx-no-literals: add ignoreProps option to ignore props validation #2146

Conversation

iiison commented Jan 29, 2019 • edited by ljharb

iiison commented Feb 7, 2019

iiison commented Feb 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iiison commented Feb 7, 2019

ljharb commented Feb 7, 2019

iiison commented Mar 27, 2019

ljharb commented Mar 27, 2019

iiison commented Apr 3, 2019

ljharb commented Apr 6, 2019

iiison commented Apr 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iiison commented Apr 25, 2019

iiison commented Apr 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iiison commented Apr 29, 2019

ljharb commented Apr 29, 2019

iiison commented Apr 29, 2019

iiison commented Apr 29, 2019

iiison commented Apr 29, 2019

ljharb commented Apr 30, 2019

iiison commented May 3, 2019

mauricecruz commented Oct 22, 2019

iiison commented Oct 30, 2019

dbassett17 commented Nov 24, 2019

Choose a reason for hiding this comment

SalimBensiali Feb 25, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljharb commented Dec 13, 2019

SalimBensiali commented Feb 24, 2020

ljharb commented Feb 25, 2020

shospodarets commented Mar 8, 2020

ljharb left a comment

Choose a reason for hiding this comment

[New]: `jsx-no-literals`: add `ignoreProps` option to ignore props validation #2146

[New]: `jsx-no-literals`: add `ignoreProps` option to ignore props validation #2146

iiison commented Jan 29, 2019 •

edited by ljharb

SalimBensiali Feb 25, 2020 •

edited