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

chore: custom ESLint rule to disable sorting for ref attribute to keep it as last property in JSX to ensure it is up-to-date #7669

Merged
merged 22 commits into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
be5b936
chore: custom ESLint rule to disable sorting for ref attribute to kee…
Elijbet Sep 3, 2023
b7fe662
Merge branch 'main' into elijbet/7659-custom-eslint-rule-enforce-ref-…
Elijbet Sep 6, 2023
92955a7
Merge branch 'main' into elijbet/7659-custom-eslint-rule-enforce-ref-…
Elijbet Oct 10, 2023
5e73981
Merge branch 'main' into elijbet/7659-custom-eslint-rule-enforce-ref-…
Elijbet Oct 11, 2023
3ea96d1
testing against a good and bad case
Elijbet Oct 11, 2023
90b2cde
type JSXAttribute, JSXSpreadAttribute, JSXOpeningElement
Elijbet Oct 11, 2023
612020c
Merge branch 'main' into elijbet/7659-custom-eslint-rule-enforce-ref-…
Elijbet Oct 11, 2023
1541324
cleanup
Elijbet Oct 11, 2023
8512f83
Merge branch 'main' into elijbet/7659-custom-eslint-rule-enforce-ref-…
Elijbet Oct 11, 2023
f9d5bd5
reword and cleanup
Elijbet Oct 11, 2023
0cd6bbc
config and docs
Elijbet Oct 12, 2023
a151897
Merge branch 'main' into elijbet/7659-custom-eslint-rule-enforce-ref-…
Elijbet Oct 12, 2023
f2ea7cc
Merge branch 'main' into elijbet/7659-custom-eslint-rule-enforce-ref-…
Elijbet Oct 12, 2023
c3b02aa
README.md
Elijbet Oct 12, 2023
ec1361b
Merge branch 'main' into elijbet/7659-custom-eslint-rule-enforce-ref-…
Elijbet Oct 12, 2023
99238fa
cleanup
Elijbet Oct 12, 2023
3ebeb9e
Merge branch 'main' into elijbet/7659-custom-eslint-rule-enforce-ref-…
Elijbet Oct 29, 2023
e9b6b54
Merge branch 'main' into elijbet/7659-custom-eslint-rule-enforce-ref-…
Elijbet Nov 2, 2023
6376f32
Provide more context to README, rule description, doc file, error mes…
Elijbet Nov 2, 2023
5a50fc6
minor tweaks in doc wording
Elijbet Nov 2, 2023
e81d735
typo
Elijbet Nov 2, 2023
74a9b65
Merge branch 'main' into elijbet/7659-custom-eslint-rule-enforce-ref-…
Elijbet Nov 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/calcite-components/.eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module.exports = {
"plugin:jsdoc/recommended",
"prettier",
],
ignorePatterns: ["dist", "docs", "hydrate", "www"],
ignorePatterns: ["dist", "docs", "hydrate", "www", ".eslintrc.cjs"],
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
parser: "@typescript-eslint/parser",
parserOptions: {
tsconfigRootDir: __dirname,
Expand Down
9 changes: 7 additions & 2 deletions packages/eslint-plugin-calcite-components/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ Add a new `lint` script to `package.json`:

Then you can run the linter:

```
```javascript
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
npm run lint
```

Expand All @@ -49,6 +49,10 @@ This rule helps prevent usage of specific events and allows suggesting alternati

This rule catches props/attributes that should be in the encapsulated HTML structure and not on the host element.

- [`@esri/calcite-components/enforce-ref-last-prop`](./docs/enforce-ref-last-prop.md)

This rule ensures the `ref` attribute is ordered last in a JSXElement to keep it up-to-date.
Elijbet marked this conversation as resolved.
Show resolved Hide resolved

- [`@esri/calcite-components/require-event-emitter-type`](./docs/require-event-emitter-type.md)

This rule helps enforce the payload type to EventEmitters to avoid misleading `any` type on the CustomEvent detail object.
Expand All @@ -62,6 +66,7 @@ This rule catches boolean props that are initialized in a way that does not conf
```json
{
"@esri/calcite-components/ban-props-on-host": "error",
"@esri/calcite-components/enforce-ref-last-prop": "error",
"@esri/calcite-components/require-event-emitter-type": "error",
"@esri/calcite-components/strict-boolean-attributes": "error"
}
Expand All @@ -83,4 +88,4 @@ See use restrictions at <http://www.esri.com/legal/pdfs/mla_e204_e300/english>

For additional information, contact: Environmental Systems Research Institute, Inc. Attn: Contracts and Legal Services Department 380 New York Street Redlands, California, USA 92373 USA

email: [email protected]
email: <[email protected]>
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# enforce-ref-last-prop

Ensures the `ref` attribute is ordered last in a JSXElement to keep it up-to-date.

## Config

No config is needed

## Usage

```json
{ "@esri/calcite-components/enforce-ref-last-prop": "error" }
```
15 changes: 8 additions & 7 deletions packages/eslint-plugin-calcite-components/src/configs/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,20 @@ export default {
ecmaVersion: 2018,
sourceType: "module",
ecmaFeatures: {
jsx: true
}
jsx: true,
},
},
env: {
es2020: true,
browser: true
browser: true,
},
plugins: ["@esri/calcite-components"],
rules: {
"@esri/calcite-components/ban-props-on-host": 2,
"@esri/calcite-components/enforce-ref-last-prop": 2,
"@esri/calcite-components/require-event-emitter-type": 2,
"@esri/calcite-components/strict-boolean-attributes": 2
}
}
]
"@esri/calcite-components/strict-boolean-attributes": 2,
},
},
],
};
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ export default {
extends: ["plugin:@esri/calcite-components/base"],
rules: {
"@esri/calcite-components/ban-props-on-host": 2,
"@esri/calcite-components/enforce-ref-last-prop": 2,
"@esri/calcite-components/require-event-emitter-type": 2,
"@esri/calcite-components/strict-boolean-attributes": 2
}
"@esri/calcite-components/strict-boolean-attributes": 2,
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { Rule } from "eslint";
import type { JSXAttribute, JSXSpreadAttribute, JSXOpeningElement } from "@babel/types";

const rule: Rule.RuleModule = {
meta: {
docs: {
description: `Ensures the "ref" attribute is ordered last in a JSXElement to keep it up-to-date.`,
category: "Best Practices",
recommended: true,
},
fixable: "code",
schema: [],
type: "problem",
},

create(context): Rule.RuleListener {
return {
JSXIdentifier(node) {
const openingElement = node.parent as JSXOpeningElement;
if (openingElement.type === "JSXOpeningElement") {
const attributes: string[] = [];

openingElement.attributes.forEach((attr: JSXAttribute | JSXSpreadAttribute) => {
if (attr.type === "JSXAttribute" && attr.name?.type === "JSXIdentifier") {
attributes.push(attr.name.name);
}
});

const refAttribute = attributes.find((attr: string) => attr === "ref");

if (refAttribute && attributes.indexOf(refAttribute) !== attributes.length - 1) {
context.report({
node,
message: `The "ref" attribute should be the last attribute in a JSXElement.`,
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
});
}
}
},
};
},
};

export default rule;
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import banEvents from "./ban-events";
import banPropsOnHost from "./ban-props-on-host";
import enforceRefLastProp from "./enforce-ref-last-prop";
import requireEventEmitterType from "./require-event-emitter-type";
import strictBooleanAttributes from "./strict-boolean-attributes";

export default {
"ban-events": banEvents,
"ban-props-on-host": banPropsOnHost,
"enforce-ref-last-prop": enforceRefLastProp,
"require-event-emitter-type": requireEventEmitterType,
"strict-boolean-attributes": strictBooleanAttributes
"strict-boolean-attributes": strictBooleanAttributes,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// @ts-nocheck
@Component({ tag: "sample-tag" })
export class SampleTag {
render() {
return (
<Host>
<div
class="some-class"
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
id={`${guid}-element`}
onClick={() => {
/* click! */
}}
ref={(el: HTMLDivElement): void => {
/* refEl */
}}
>
test
</div>
</Host>
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import rule from "../../../../src/rules/enforce-ref-last-prop";
import { ruleTester } from "stencil-eslint-core";
import * as path from "path";
import * as fs from "fs";

const projectPath = path.resolve(__dirname, "../../../tsconfig.json");

describe("enforce-ref-last-prop rule", () => {
const files = {
good: path.resolve(__dirname, "enforce-ref-last-prop.good.tsx"),
wrong: path.resolve(__dirname, "enforce-ref-last-prop.wrong.tsx"),
};
ruleTester(projectPath).run("enforce-ref-last-prop", rule, {
valid: [
{
code: fs.readFileSync(files.good, "utf8"),
filename: files.good,
},
],

invalid: [
{
code: fs.readFileSync(files.wrong, "utf8"),
filename: files.wrong,
errors: 1,
},
],
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// @ts-nocheck
@Component({ tag: "sample-tag" })
export class SampleTag {
render() {
return (
<Host>
<div
ref={(el: HTMLDivElement): void => {
/* refEl */
}}
class="some-class"
id={`${guid}-element`}
onClick={() => {
/* click! */
}}
Copy link
Member

Choose a reason for hiding this comment

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

tabIndex should go here to match the good case.

>
test
</div>
</Host>
);
}
}
Loading