Update @angular-eslint/template/i18n to ignore common nimble and sl-lib attributes (#140)

## Justification

Update `@angular-eslint/template/i18n` to ignore common nimble and
sl-lib attributes (and excludes tests of the name `*.spec.ts` with
inline templates).

fixes #83

## Implementation

A new file `template/options.js` is added to list all well-known
attributes that should not be localized for using in NI javascript
codebases. This includes common nimble attributes,
systemlink-lib-angular attributes, and attributes from commonly used
third-party libraries.

This list is used to modify the `@angular-eslint/template/i18n` eslint
rule to ignore any attribute in the list by default. If apps need to
change this to ignore only nimble attributes, they have the option to do
that by overriding the rule's config in their app's eslintrc.

## Testing

- I have made modifications to the angular test project to include
attributes on the two sample templates that should either be ignored or
not ignored and verified the eslint rule modification is being correctly
applied in each.
- Added a test to verify custom attributes can be added to ignore lists
by apps.

---------

Co-authored-by: rajsite <[email protected]>

Author: rbell517

CONTRIBUTING.md

@@ -42,7 +42,7 @@ The goal of the smoke tests is basic validation that the eslint configurations c
 1. Create a fork of the repository.
 2. (optional, but recommended) On the Actions tab of your repository enable actions for the repo. This allows the tests to run in the branches of your fork.
 3. Create and edit a branch in your fork.
-4. Open a pull request to Nimble for the branch.
+4. Open a pull request to `@ni/javascript-styleguide` for the branch.
    - (optional, but recommended) While creating the pull request enable the "Allow edits and access to secrets by maintainers" option. This will allow maintainers to push small changes to the branch to resolve needed changes quicker.
 5. Create a beachball change file for your project by doing one of the following:
    1. Follow the [Beachball change file](#beachball-change-file) instructions below to create a change file and push to your branch.

change/@ni-eslint-config-angular-ff70c9b2-14ea-4eb3-9c0e-0de3cdb388ec.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Update @angular-eslint/template/i18n to ignore common nimble and sl-lib attributes",
+  "packageName": "@ni/eslint-config-angular",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

packages/eslint-config-angular/package.json

@@ -27,8 +27,9 @@
     "access": "public"
   },
   "files": [
-    "/*.js",
-    "!/.*.js"
+    "**/*.js",
+    "!/.*.js",
+    "!/tools"
   ],
   "peerDependencies": {
     "@angular-eslint/builder": "^16.3.1",

packages/eslint-config-angular/template.js

@@ -1,3 +1,5 @@
+const { ignoreAttributes } = require('./template/options');
+
 module.exports = {
     extends: [
         'plugin:@angular-eslint/template/recommended'
@@ -68,7 +70,13 @@ module.exports = {
             and new applications in the chance that they'll need to be localized in the future. Disable this rule
             if an application will never be localized.
         */
-        '@angular-eslint/template/i18n': ['error', { checkId: false }],
+        '@angular-eslint/template/i18n': [
+            'error',
+            {
+                checkId: false,
+                ignoreAttributes: [...ignoreAttributes.all]
+            }
+        ],
 
         '@angular-eslint/template/no-any': 'error',
 
@@ -86,5 +94,18 @@ module.exports = {
             Providing a `trackBy` function in `ngFor` loops can improve performance in specific cases where Angular can't track references, but it's overkill to require it for every `ngFor` so this rule is disabled.
         */
         '@angular-eslint/template/use-track-by-function': 'off'
-    }
+    },
+    overrides: [
+        {
+            // Ignore inline templates in tests using the inline template naming convention
+            // See naming details: https://github.com/angular-eslint/angular-eslint/releases/tag/v14.0.0
+            files: ['*.spec.ts*.html'],
+            rules: {
+                /*
+                    Tests often define helper components that don't need to be marked for i18n.
+                */
+                '@angular-eslint/template/i18n': 'off'
+            }
+        },
+    ]
 };

packages/eslint-config-angular/template/options.js

@@ -0,0 +1,88 @@
+// This is a list of standard element attributes to ignore
+// when applying the @angular-eslint/template/i18n rule
+// that requires attributes to be tagged as localizable.
+//
+// New attributes can be added here that are for element
+// configuration and not for display, where localizing
+// the attribute value would result in broken functionality.
+//
+// In cases of attribute collisions, the rule supports
+// CSS selector style element[attribute] ignore entries
+// which can be used to scope attribute ignores to specific
+// elements.
+const ignoreAttributeSets = {
+    nimble: [
+        'action-menu-slot',
+        'activeid',
+        'appearance',
+        'appearance-variant',
+        'autocomplete',
+        'column-id',
+        'field-name',
+        'format',
+        'icon',
+        'id-field-name',
+        'key',
+        'key-type',
+        'nimbleRouterLink',
+        'orientation',
+        'queryParamsHandling',
+        'resize',
+        'selection-mode',
+        'severity',
+        'slot',
+        'theme'
+    ],
+    systemlink: [
+        // sl-workspace-selector
+        'action',
+
+        // sl-table
+        'columnId',
+        'decimalDigits',
+        'fieldName',
+        'format',
+        'hrefFieldName',
+        'idFieldName',
+        'keyType',
+        'labelFieldName',
+        'selectionMode',
+        'slTableColumnId',
+
+        // sl-grid
+        'columnSizeMode',
+        'dataRowId',
+        'dataSourceFingerprint',
+        'gridId',
+        'loadColumnStateBehavior',
+        'parentDataField',
+
+        // sl-query-builder
+        'dropdownWidth',
+    ],
+    jqx: [
+        // smart-table
+        'sortMode',
+
+        // smart-query-builder
+        'applyMode',
+        'fieldsMode'
+    ],
+    material: [
+        'cdkDragPreviewContainer',
+        'floatLabel',
+        'fontIcon',
+        'fontSet',
+        'matColumnDef',
+        'matTooltipClass',
+    ]
+};
+
+const ignoreAttributes = {
+    ...ignoreAttributeSets,
+    all: Object.values(ignoreAttributeSets).flat()
+};
+
+module.exports = {
+    ignoreAttributes
+};

tests/angular/.eslintrc.js

@@ -1,5 +1,6 @@
 // Test TypeScript and templates together to process inline templates.
 module.exports = {
+    ignorePatterns: ['*.js'],
     overrides: [{
         extends: [
             '@ni/eslint-config-angular',

tests/angular/custom-ignore-attributes/.eslintrc.js

@@ -0,0 +1,21 @@
+const {ignoreAttributes} = require('@ni/eslint-config-angular/template/options');
+
+module.exports = {
+    overrides: [{
+        files: ['*.html'],
+        rules: {
+            '@angular-eslint/template/i18n': [
+                'error',
+                {
+                    checkId: false,
+                    ignoreAttributes: [...ignoreAttributes.all, 'custom-field']
+                }
+            ],
+        }
+    }, {
+        files: ['*.spec.ts*.html'],
+        rules: {
+            '@angular-eslint/template/i18n': 'off'
+        }
+    }]
+};

tests/angular/custom-ignore-attributes/index.html

@@ -0,0 +1,2 @@
+<!-- Angular Template Custom Ignore Attributes Test -->
+<span i18n custom-field="test-custom-field">Hello {{name}}</span>

tests/angular/custom-ignore-attributes/index.spec.ts

@@ -0,0 +1,24 @@
+// Angular Test Spec Custom Ignore Attributes Test
+import { Component, Input, ViewChild } from '@angular/core';
+import { ComponentFixture } from '@angular/core/testing';
+
+@Component({
+    template: '<div custom-attribute="i18n-should-be-ignored-in-test">missing i18n should be ignored</div>'
+})
+class MyComponent {
+    @Input() public attr = false;
+    @ViewChild('div') public div: HTMLDivElement;
+    public myMethod(): void {}
+}
+
+describe('MyComponent', () => {
+    let hostComponent: MyComponent;
+    let fixture: ComponentFixture<MyComponent>;
+
+    it('should have a div', async () => {
+        await fixture.whenStable();
+
+        expect(hostComponent.div).toBeDefined();
+        expect(hostComponent.myMethod).not.toHaveBeenCalled();
+    });
+});

tests/angular/custom-ignore-attributes/index.ts

@@ -0,0 +1,15 @@
+// Angular TypeScript and Inline Template Custom Ignore Attributes Test
+import { Component, Input } from '@angular/core';
+
+@Component({
+    selector: 'app-root',
+    template: '<span i18n custom-field="test-custom-field">Hello {{name}}</span>!',
+    styles: [`
+        span {
+            font-weight: bold;
+        }
+    `]
+})
+export class AppComponent {
+    @Input() public name = 'Angular';
+}

tests/angular/index.html

@@ -1,2 +1,2 @@
 <!-- Angular Template Smoke Test -->
-<span i18n [(id)]="name">Hello {{ name }}</span>!
+<span i18n [(id)]="name" theme="non-localized-theme-id" title="This title should be localized." i18n-title>Hello {{name}}</span>

tests/angular/index.spec.ts

@@ -3,7 +3,7 @@ import { Component, Input, ViewChild } from '@angular/core';
 import { ComponentFixture } from '@angular/core/testing';
 
 @Component({
-    template: '<div [(attr)] = true></div>'
+    template: '<div custom-attribute="i18n-should-be-ignored-in-test">missing i18n should be ignored</div>'
 })
 class MyComponent {
     @Input() public attr = false;

tests/angular/index.ts

@@ -3,7 +3,7 @@ import { Component, Input } from '@angular/core';
 
 @Component({
     selector: 'app-root',
-    template: '<span i18n [(id)]="name">Hello {{name}}</span>!',
+    template: '<span i18n [(id)]="name" theme="non-localized-theme-id" title="This title should be localized." i18n-title>Hello {{name}}</span>!',
     styles: [`
         span {
             font-weight: bold;

tests/angular/tsconfig.json

@@ -2,8 +2,5 @@
     "compilerOptions": {
         "experimentalDecorators": true
     },
-    "files": [
-        "index.ts",
-        "index.spec.ts"
-    ]
+    "include": ["."]
 }