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

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

Merged
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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": "418101+rbell517@users.noreply.github.com",
"dependentChangeType": "patch"
}
5 changes: 3 additions & 2 deletions packages/eslint-config-angular/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,9 @@
"access": "public"
},
"files": [
"/*.js",
"!/.*.js"
"**/*.js",
"!/.*.js",
"!/tools"
],
"peerDependencies": {
"@angular-eslint/builder": "^16.3.1",
Expand Down
25 changes: 23 additions & 2 deletions packages/eslint-config-angular/template.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const { ignoreAttributes } = require('./template/options');

module.exports = {
extends: [
'plugin:@angular-eslint/template/recommended'
Expand Down Expand Up @@ -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',

Expand All @@ -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: [
rajsite marked this conversation as resolved.
Show resolved Hide resolved
{
// 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'
}
},
]
};
88 changes: 88 additions & 0 deletions packages/eslint-config-angular/template/options.js
Original file line number Diff line number Diff line change
@@ -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
};
1 change: 1 addition & 0 deletions tests/angular/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Test TypeScript and templates together to process inline templates.
module.exports = {
ignorePatterns: ['*.js'],
overrides: [{
extends: [
'@ni/eslint-config-angular',
Expand Down
21 changes: 21 additions & 0 deletions tests/angular/custom-ignore-attributes/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -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'
}
}]
rajsite marked this conversation as resolved.
Show resolved Hide resolved
};
2 changes: 2 additions & 0 deletions tests/angular/custom-ignore-attributes/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<!-- Angular Template Custom Ignore Attributes Test -->
<span i18n custom-field="test-custom-field">Hello {{name}}</span>
24 changes: 24 additions & 0 deletions tests/angular/custom-ignore-attributes/index.spec.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
15 changes: 15 additions & 0 deletions tests/angular/custom-ignore-attributes/index.ts
Original file line number Diff line number Diff line change
@@ -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';
}
2 changes: 1 addition & 1 deletion tests/angular/index.html
Original file line number Diff line number Diff line change
@@ -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>
2 changes: 1 addition & 1 deletion tests/angular/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion tests/angular/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 1 addition & 4 deletions tests/angular/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,5 @@
"compilerOptions": {
"experimentalDecorators": true
},
"files": [
"index.ts",
"index.spec.ts"
]
"include": ["."]
}