Skip to content

Commit

Permalink
resolved pr comments related to docs and rule
Browse files Browse the repository at this point in the history
  • Loading branch information
@etherealm13
etherealm13 committed Jul 26, 2024
1 parent 493e23a commit 580b303
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 89 deletions.
83 changes: 45 additions & 38 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,47 +34,47 @@ You should also specify settings that will be shared across all the plugin rules

```json5
{
settings: {
react: {
createClass: 'createReactClass', // Regex for Component Factory to use,
// default to "createReactClass"
pragma: 'React', // Pragma to use, default to "React"
fragment: 'Fragment', // Fragment to use (may be a property of <pragma>), default to "Fragment"
version: 'detect', // React version. "detect" automatically picks the version you have installed.
// You can also use `16.0`, `16.3`, etc, if you want to override the detected value.
// Defaults to the "defaultVersion" setting and warns if missing, and to "detect" in the future
defaultVersion: '', // Default React version to use when the version you have installed cannot be detected.
// If not provided, defaults to the latest React version.
flowVersion: '0.53', // Flow version
"settings": {
"react": {
"createClass": "createReactClass", // Regex for Component Factory to use,
// default to "createReactClass"
"pragma": "React", // Pragma to use, default to "React"
"fragment": "Fragment", // Fragment to use (may be a property of <pragma>), default to "Fragment"
"version": "detect", // React version. "detect" automatically picks the version you have installed.
// You can also use `16.0`, `16.3`, etc, if you want to override the detected value.
// Defaults to the "defaultVersion" setting and warns if missing, and to "detect" in the future
"defaultVersion": "", // Default React version to use when the version you have installed cannot be detected.
// If not provided, defaults to the latest React version.
"flowVersion": "0.53" // Flow version
},
propWrapperFunctions: [
// The names of any function used to wrap propTypes, e.g. `forbidExtraProps`. If this isn't set, any propTypes wrapped in a function will be skipped.
'forbidExtraProps',
{ property: 'freeze', object: 'Object' },
{ property: 'myFavoriteWrapper' },
// for rules that check exact prop wrappers
{ property: 'forbidExtraProps', exact: true },
"propWrapperFunctions": [
// The names of any function used to wrap propTypes, e.g. `forbidExtraProps`. If this isn't set, any propTypes wrapped in a function will be skipped.
"forbidExtraProps",
{"property": "freeze", "object": "Object"},
{"property": "myFavoriteWrapper"},
// for rules that check exact prop wrappers
{"property": "forbidExtraProps", "exact": true}
],
componentWrapperFunctions: [
// The name of any function used to wrap components, e.g. Mobx `observer` function. If this isn't set, components wrapped by these functions will be skipped.
'observer', // `property`
{ property: 'styled' }, // `object` is optional
{ property: 'observer', object: 'Mobx' },
{ property: 'observer', object: '<pragma>' }, // sets `object` to whatever value `settings.react.pragma` is set to
"componentWrapperFunctions": [
// The name of any function used to wrap components, e.g. Mobx `observer` function. If this isn't set, components wrapped by these functions will be skipped.
"observer", // `property`
{"property": "styled"}, // `object` is optional
{"property": "observer", "object": "Mobx"},
{"property": "observer", "object": "<pragma>"} // sets `object` to whatever value `settings.react.pragma` is set to
],
formComponents: [
"formComponents": [
// Components used as alternatives to <form> for forms, eg. <Form endpoint={ url } />
'CustomForm',
{ name: 'SimpleForm', formAttribute: 'endpoint' },
{ name: 'Form', formAttribute: ['registerEndpoint', 'loginEndpoint'] }, // allows specifying multiple properties if necessary
"CustomForm",
{"name": "SimpleForm", "formAttribute": "endpoint"},
{"name": "Form", "formAttribute": ["registerEndpoint", "loginEndpoint"]}, // allows specifying multiple properties if necessary
],
linkComponents: [
"linkComponents": [
// Components used as alternatives to <a> for linking, eg. <Link to={ url } />
'Hyperlink',
{ name: 'MyLink', linkAttribute: 'to' },
{ name: 'Link', linkAttribute: ['to', 'href'] }, // allows specifying multiple properties if necessary
],
},
"Hyperlink",
{"name": "MyLink", "linkAttribute": "to"},
{"name": "Link", "linkAttribute": ["to", "href"]}, // allows specifying multiple properties if necessary
]
}
}
```

Expand All @@ -84,7 +84,9 @@ Add "react" to the plugins section.

```json
{
"plugins": ["react"]
"plugins": [
"react"
]
}
```

Expand Down Expand Up @@ -134,7 +136,9 @@ This pairs well with the `eslint:all` rule.

```json
{
"plugins": ["react"],
"plugins": [
"react"
],
"extends": ["eslint:all", "plugin:react/all"]
}
```
Expand Down Expand Up @@ -201,7 +205,6 @@ Refer to the [official docs](https://eslint.org/docs/latest/user-guide/configuri
The schema of the `settings.react` object would be identical to that of what's already described above in the legacy config section.

<!-- markdownlint-disable-next-line no-duplicate-heading -->

### Flat Configs

This plugin exports 3 flat configs:
Expand Down Expand Up @@ -405,11 +408,15 @@ module.exports = [

[npm-url]: https://npmjs.org/package/eslint-plugin-react
[npm-image]: https://img.shields.io/npm/v/eslint-plugin-react.svg

[status-url]: https://github.com/jsx-eslint/eslint-plugin-react/pulse
[status-image]: https://img.shields.io/github/last-commit/jsx-eslint/eslint-plugin-react.svg

[tidelift-url]: https://tidelift.com/subscription/pkg/npm-eslint-plugin-react?utm_source=npm-eslint-plugin-react&utm_medium=referral&utm_campaign=readme
[tidelift-image]: https://tidelift.com/badges/package/npm/eslint-plugin-react?style=flat

[package-url]: https://npmjs.org/package/eslint-plugin-react
[npm-version-svg]: https://versionbadg.es/jsx-eslint/eslint-plugin-react.svg

[actions-image]: https://img.shields.io/endpoint?url=https://github-actions-badge-u3jn4tfpocch.runkit.sh/jsx-eslint/eslint-plugin-react
[actions-url]: https://github.com/jsx-eslint/eslint-plugin-react/actions
39 changes: 7 additions & 32 deletions docs/rules/padding-lines-between-tags.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Require or disallow newlines between sibling tags in React.

This rule requires blank lines between each sibling HTML tag by default.

A configuration is an object which has 3 properties; `blankLine`, `prev` and `next`. For example, `{ blankLine: "always", prev: "br", next: "div" }` means “one or more blank lines are required between a `br` tag and a `div` tag.” You can supply any number of configurations. If a tag pair matches multiple configurations, the last matched configuration will be used.
A configuration is an object which has 3 properties; `blankLine`, `prev`, and `next`. For example, `{ blankLine: "always", prev: "br", next: "div" }` means “one or more blank lines are required between a `br` tag and a `div` tag.” You can supply any number of configurations. If a tag pair matches multiple configurations, the last matched configuration will be used.

- `blankLine` is one of the following:
- `always` requires one or more blank lines.
Expand All @@ -32,11 +32,7 @@ A configuration is an object which has 3 properties; `blankLine`, `prev` and `ne

`{ blankLine: 'never', prev: '*', next: '*' }`

<eslint-code-block fix :rules="{'padding-line-between-tags': ['error', [
{ blankLine: 'never', prev: '*', next: '*' }
]]}">

```react
```jsx
<App>
<div>
<div></div>
Expand All @@ -47,17 +43,11 @@ A configuration is an object which has 3 properties; `blankLine`, `prev` and `ne
</App>
```

</eslint-code-block>

### Require newlines after `<br>`

`{ blankLine: 'always', prev: 'br', next: '*' }`

<eslint-code-block fix :rules="{'react/padding-line-between-tags': ['error', [
{ blankLine: 'always', prev: 'br', next: '*' }
]]}">

```react
```jsx
<App>
<div>
<ul>
Expand All @@ -72,18 +62,11 @@ A configuration is an object which has 3 properties; `blankLine`, `prev` and `ne
</App>
```

</eslint-code-block>

### Require newlines between `<br>` and `<img>`

`{ blankLine: 'always', prev: 'br', next: 'img' }`

<eslint-code-block fix :rules="{'react/padding-line-between-tags': ['error', [
{ blankLine: 'always', prev: 'br', next: 'img' },
{ blankLine: 'always', prev: 'li', next: 'li' },
]]}">

```react
```jsx
<App>
<div>
<ul>
Expand All @@ -100,7 +83,7 @@ A configuration is an object which has 3 properties; `blankLine`, `prev` and `ne
</App>
```

```react [Fixed]
```jsx [Fixed]
<App>
<div>
<ul>
Expand All @@ -119,17 +102,11 @@ A configuration is an object which has 3 properties; `blankLine`, `prev` and `ne
</App>
```

</eslint-code-block>

### Require consistent newlines

`{ blankLine: 'consistent', prev: '*', next: '*' }`

<eslint-code-block fix :rules="{'react/padding-line-between-tags': ['error', [
{ blankLine: 'consistent', prev: '*', next: '*' }
]]}">

```react
```jsx
<App>
<div>
<ul>
Expand All @@ -145,8 +122,6 @@ A configuration is an object which has 3 properties; `blankLine`, `prev` and `ne
</App>
```

</eslint-code-block>

## When Not To Use It

If you are not using React.
If you are not using jsx.
36 changes: 17 additions & 19 deletions lib/rules/padding-lines-between-tags.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,42 +8,40 @@
'use strict';

const docsUrl = require('../util/docsUrl');
const report = require('../util/report');
const eslintUtil = require('../util/eslint');

/**
* Split the source code into multiple lines based on the line delimiters.
* Copied from padding-line-between-blocks
* @param {string} text Source code as a string.
* @returns {string[]} Array of source code lines.
*/
const messages = {
never: 'Unexpected blank line before this tag.',
always: 'Expected blank line before this tag.',
};

function splitLines(text) {
return text.split(/\r\n|[\r\n\u2028\u2029]/gu);
}

function insertNewLine(context, tag, sibling, lineDifference) {
function insertNewLine(tag, sibling, lineDifference) {
const endTag = tag.closingElement || tag.openingElement;

if (lineDifference === 1) {
context.report({
messageId: 'always',
report({
message: 'Unexpected blank line before {{endTag}}.',
loc: sibling && sibling.loc,
// @ts-ignore
fix(fixer) {
return fixer.insertTextAfter(tag, '\n');
},
});
} else if (lineDifference === 0) {
context.report({
messageId: 'always',
report({
message: 'Expected blank line before {{endTag}}.',
loc: sibling && sibling.loc,
// @ts-ignore
fix(fixer) {
const lastSpaces = /** @type {RegExpExecArray} */ (
/^\s*/.exec(context.getSourceCode().lines[endTag.loc.start.line - 1])
/^\s*/.exec(eslintUtil.getSourceCode().lines[endTag.loc.start.line - 1])
)[0];

return fixer.insertTextAfter(endTag, `\n\n${lastSpaces}`);
Expand All @@ -52,25 +50,25 @@ function insertNewLine(context, tag, sibling, lineDifference) {
}
}

function removeExcessLines(context, endTag, sibling, lineDifference) {
function removeExcessLines(endTag, sibling, lineDifference) {
if (lineDifference > 1) {
let hasOnlyTextBetween = true;
for (
let i = endTag.loc && endTag.loc.start.line;
i < sibling.loc.start.line - 1 && hasOnlyTextBetween;
i++
) {
hasOnlyTextBetween = !/^\s*$/.test(context.getSourceCode().lines[i]);
hasOnlyTextBetween = !/^\s*$/.test(eslintUtil.getSourceCode().lines[i]);
}
if (!hasOnlyTextBetween) {
context.report({
report({
messageId: 'never',
loc: sibling && sibling.loc,
// @ts-ignore
fix(fixer) {
const start = endTag.range[1];
const end = sibling.range[0];
const paddingText = context.getSourceCode().text.slice(start, end);
const paddingText = eslintUtil.getSourceCode().text.slice(start, end);
const textBetween = splitLines(paddingText);
let newTextBetween = `\n${textBetween.pop()}`;
for (let i = textBetween.length - 1; i >= 0; i--) {
Expand All @@ -87,7 +85,7 @@ function removeExcessLines(context, endTag, sibling, lineDifference) {
}
}

function checkNewLine(context, configureList) {
function checkNewLine(configureList) {
const firstConsistentBlankLines = new Map();

const reverseConfigureList = [...configureList].reverse();
Expand Down Expand Up @@ -143,15 +141,16 @@ function checkNewLine(context, configureList) {
}

if (blankLine === 'always') {
insertNewLine(context, node, closestSibling, lineDifference);
insertNewLine(node, closestSibling, lineDifference);
} else {
removeExcessLines(context, endTag, closestSibling, lineDifference);
removeExcessLines(endTag, closestSibling, lineDifference);
}
};
}

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
// eslint-disable-next-line eslint-plugin/prefer-message-ids
meta: {
docs: {
description: 'Enforce no padding lines between tags for React Components',
Expand All @@ -160,7 +159,6 @@ module.exports = {
url: docsUrl('padding-lines-between-tags'),
},
fixable: 'code',
messages,
schema: [
{
type: 'array',
Expand All @@ -183,7 +181,7 @@ module.exports = {
{ blankLine: 'always', prev: '*', next: '*' },
];
return {
JSXElement: checkNewLine(context, configureList),
JSXElement: checkNewLine(configureList),
};
},
};

0 comments on commit 580b303

Please sign in to comment.