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

fix(angular-eslint): ignore hostDirectives for no-output-rename and no-outputs-metadata-property #1466

99 changes: 99 additions & 0 deletions packages/eslint-plugin/docs/rules/no-output-rename.md
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
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
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
@@ -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
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
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