Skip to content

Commit

Permalink
chore: custom ESLint rule to disable sorting for ref attribute to k…
Browse files Browse the repository at this point in the history
…eep it as last property in JSX to ensure it is up-to-date (#7669)

**Related Issue:** #7659

## Summary
Custom ESLint rule to provide a more granular prop ordering mechanism by
forcing the `ref` attribute to be kept as the last property so we can
ensure it is up-to-date.

Added a test file for the new custom rules: the file references
different use cases, which are 1 good and 1 bad usage files.
  • Loading branch information
Elijbet authored Nov 2, 2023
1 parent d9615c6 commit 589d09f
Show file tree
Hide file tree
Showing 9 changed files with 153 additions and 12 deletions.
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:

```
```shell
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 this is a workaround for a [Stencil bug](https://github.com/ionic-team/stencil/issues/4074) where ref is called in the specified order and not after initializing element with all its attributes/properties. This can cause attributes/properties to be outdated by the time the callback is invoked. This rule ensures the `ref` attribute is ordered last in a JSXElement to keep it up-to-date.

- [`@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,15 @@
# enforce-ref-last-prop

This updates `ref` usage to work around a Stencil bug where ref is called in the specified order and not after initializing element with all its attributes/properties. This can cause attributes/properties to be outdated by the time the callback is invoked.

This rule 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,42 @@
import { Rule } from "eslint";
import type { JSXAttribute, JSXSpreadAttribute, JSXOpeningElement } from "@babel/types";

const rule: Rule.RuleModule = {
meta: {
docs: {
description: `This updates ref usage to work around a Stencil bug where ref is called in the specified order and not after initializing element with all its attributes/properties. This can cause attributes/properties to be outdated by the time the callback is invoked. This rule ensures the ref attribute is ordered last in a JSXElement to keep it up-to-date.`,
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: `Attribute "ref" should be placed last in a JSXElement so node attrs/props are in sync. If it's called in the specified order, attributes/properties can be outdated by the time the callback is invoked.`,
});
}
}
},
};
},
};

export default rule;
4 changes: 3 additions & 1 deletion packages/eslint-plugin-calcite-components/src/rules/index.ts
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,23 @@
// @ts-nocheck
@Component({ tag: "sample-tag" })
export class SampleTag {
render() {
return (
<Host>
<div
class="some-class"
id={`${guid}-element`}
onClick={() => {
/* click! */
}}
tabIndex={0}
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,23 @@
// @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! */
}}
tabIndex={0}
>
test
</div>
</Host>
);
}
}

0 comments on commit 589d09f

Please sign in to comment.