Skip to content

Commit

Permalink
fix(angular-eslint): ignore hostDirectives for no-output-rename and n…
Browse files Browse the repository at this point in the history
…o-outputs-metadata-property (#1466)
  • Loading branch information
profanis committed Aug 21, 2023
1 parent 471d6ad commit 208bf25
Show file tree
Hide file tree
Showing 6 changed files with 315 additions and 1 deletion.
99 changes: 99 additions & 0 deletions packages/eslint-plugin/docs/rules/no-output-rename.md
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,105 @@ class Test {

#### ✅ Valid Code

```ts
@Component({
selector: 'foo',
hostDirectives: [{
directive: CdkMenuItem,
outputs: ['cdkMenuItemTriggered: triggered'],
}]
})
class Test {}
```

<br>

---

<br>

#### Default Config

```json
{
"rules": {
"@angular-eslint/no-output-rename": [
"error"
]
}
}
```

<br>

#### ✅ Valid Code

```ts
@Component({
selector: 'foo',
'hostDirectives': [{
directive: CdkMenuItem,
outputs: ['cdkMenuItemTriggered: triggered'],
}]
})
class Test {}
```

<br>

---

<br>

#### Default Config

```json
{
"rules": {
"@angular-eslint/no-output-rename": [
"error"
]
}
}
```

<br>

#### ✅ Valid Code

```ts
@Component({
selector: 'foo',
['hostDirectives']: [{
directive: CdkMenuItem,
outputs: ['cdkMenuItemTriggered: triggered'],
}]
})
class Test {}
```

<br>

---

<br>

#### Default Config

```json
{
"rules": {
"@angular-eslint/no-output-rename": [
"error"
]
}
}
```

<br>

#### ✅ Valid Code

```ts
@Directive({
selector: 'foo'
Expand Down
99 changes: 99 additions & 0 deletions packages/eslint-plugin/docs/rules/no-outputs-metadata-property.md
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,105 @@ class Test {}
class Test {}
```

<br>

---

<br>

#### Default Config

```json
{
"rules": {
"@angular-eslint/no-outputs-metadata-property": [
"error"
]
}
}
```

<br>

#### ✅ Valid Code

```ts
@Component({
selector: 'foo',
hostDirectives: [{
directive: CdkMenuItem,
outputs: ['cdkMenuItemTriggered: triggered'],
}]
})
class Test {}
```

<br>

---

<br>

#### Default Config

```json
{
"rules": {
"@angular-eslint/no-outputs-metadata-property": [
"error"
]
}
}
```

<br>

#### ✅ Valid Code

```ts
@Component({
selector: 'foo',
'hostDirectives': [{
directive: CdkMenuItem,
outputs: ['cdkMenuItemTriggered: triggered'],
}]
})
class Test {}
```

<br>

---

<br>

#### Default Config

```json
{
"rules": {
"@angular-eslint/no-outputs-metadata-property": [
"error"
]
}
}
```

<br>

#### ✅ Valid Code

```ts
@Component({
selector: 'foo',
['hostDirectives']: [{
directive: CdkMenuItem,
outputs: ['cdkMenuItemTriggered: triggered'],
}]
})
class Test {}
```

</details>

<br>
23 changes: 23 additions & 0 deletions packages/eslint-plugin/src/rules/no-output-rename.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,29 @@ export default createESLintRule<Options, MessageIds>({
[Selectors.OUTPUTS_METADATA_PROPERTY_LITERAL](
node: TSESTree.Literal | TSESTree.TemplateElement,
) {
const ancestorMaybeHostDirectiveAPI =
node.parent?.parent?.parent?.parent?.parent;
if (
ancestorMaybeHostDirectiveAPI &&
ASTUtils.isProperty(ancestorMaybeHostDirectiveAPI)
) {
/**
* Angular v15 introduced the directive composition API: https://angular.io/guide/directive-composition-api
* Renaming host directive outputs using this API is not a bad practice and should not be reported
*/
const hostDirectiveAPIPropertyName = 'hostDirectives';
if (
(ASTUtils.isLiteral(ancestorMaybeHostDirectiveAPI.key) &&
ancestorMaybeHostDirectiveAPI.key.value ===
hostDirectiveAPIPropertyName) ||
(TSESLintASTUtils.isIdentifier(ancestorMaybeHostDirectiveAPI.key) &&
ancestorMaybeHostDirectiveAPI.key.name ===
hostDirectiveAPIPropertyName)
) {
return;
}
}

const [propertyName, aliasName] = withoutBracketsAndWhitespaces(
ASTUtils.getRawText(node),
).split(':');
Expand Down
27 changes: 26 additions & 1 deletion packages/eslint-plugin/src/rules/no-outputs-metadata-property.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Selectors } from '@angular-eslint/utils';
import { ASTUtils, Selectors } from '@angular-eslint/utils';
import type { TSESTree } from '@typescript-eslint/utils';
import { ASTUtils as TSESLintASTUtils } from '@typescript-eslint/utils';
import { createESLintRule } from '../utils/create-eslint-rule';

type Options = [];
Expand Down Expand Up @@ -29,6 +30,30 @@ export default createESLintRule<Options, MessageIds>({
} ${Selectors.metadataProperty(METADATA_PROPERTY_NAME)}`](
node: TSESTree.Property,
) {
/**
* Angular v15 introduced the directive composition API: https://angular.io/guide/directive-composition-api
* Using host directive outputs using this API is not a bad practice and should not be reported
*/
const ancestorMayBeHostDirectiveAPI = node.parent?.parent?.parent;

if (
ancestorMayBeHostDirectiveAPI &&
ASTUtils.isProperty(ancestorMayBeHostDirectiveAPI)
) {
const hostDirectiveAPIPropertyName = 'hostDirectives';

if (
(ASTUtils.isLiteral(ancestorMayBeHostDirectiveAPI.key) &&
ancestorMayBeHostDirectiveAPI.key.value ===
hostDirectiveAPIPropertyName) ||
(TSESLintASTUtils.isIdentifier(ancestorMayBeHostDirectiveAPI.key) &&
ancestorMayBeHostDirectiveAPI.key.name ===
hostDirectiveAPIPropertyName)
) {
return;
}
}

context.report({
node,
messageId: 'noOutputsMetadataProperty',
Expand Down
34 changes: 34 additions & 0 deletions packages/eslint-plugin/tests/rules/no-output-rename/cases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,40 @@ export const valid = [
@Output('foo') label: string;
}
`,
/**
* Renaming outputs when using the directive composition API is not a bad practice
* https://angular.io/guide/directive-composition-api
*/
`
@Component({
selector: 'foo',
hostDirectives: [{
directive: CdkMenuItem,
outputs: ['cdkMenuItemTriggered: triggered'],
}]
})
class Test {}
`,
`
@Component({
selector: 'foo',
'hostDirectives': [{
directive: CdkMenuItem,
outputs: ['cdkMenuItemTriggered: triggered'],
}]
})
class Test {}
`,
`
@Component({
selector: 'foo',
['hostDirectives']: [{
directive: CdkMenuItem,
outputs: ['cdkMenuItemTriggered: triggered'],
}]
})
class Test {}
`,
`
@Directive({
selector: 'foo'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,40 @@ export const valid = [
})
class Test {}
`,
/**
* Renaming outputs when using the directive composition API is not a bad practice
* https://angular.io/guide/directive-composition-api
*/
`
@Component({
selector: 'foo',
hostDirectives: [{
directive: CdkMenuItem,
outputs: ['cdkMenuItemTriggered: triggered'],
}]
})
class Test {}
`,
`
@Component({
selector: 'foo',
'hostDirectives': [{
directive: CdkMenuItem,
outputs: ['cdkMenuItemTriggered: triggered'],
}]
})
class Test {}
`,
`
@Component({
selector: 'foo',
['hostDirectives']: [{
directive: CdkMenuItem,
outputs: ['cdkMenuItemTriggered: triggered'],
}]
})
class Test {}
`,
];

export const invalid = [
Expand Down

0 comments on commit 208bf25

Please sign in to comment.