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 all 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
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;
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"
Elijbet marked this conversation as resolved.
Show resolved Hide resolved
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! */
}}
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.

tabIndex={0}
>
test
</div>
</Host>
);
}
}
Loading