From 24edb059e58d8b8e613eee12004da5cb3e4fb3a9 Mon Sep 17 00:00:00 2001 From: Jesse Attas Date: Thu, 19 Sep 2024 16:12:20 -0500 Subject: [PATCH 01/11] Enable no-floating-promises for Angular tests --- packages/eslint-config-angular/requiring-type-checking.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/eslint-config-angular/requiring-type-checking.js b/packages/eslint-config-angular/requiring-type-checking.js index 9e590a9..755c174 100644 --- a/packages/eslint-config-angular/requiring-type-checking.js +++ b/packages/eslint-config-angular/requiring-type-checking.js @@ -7,11 +7,6 @@ module.exports = { { files: ['*.spec.ts'], rules: { - /* - The jasminewd2 library used by Angular applications results in a significant number of - floating promises and unbound methods so these rules are disabled for test specs in Angular projects. - */ - '@typescript-eslint/no-floating-promises': 'off', '@typescript-eslint/unbound-method': 'off', } }, From fc777e89d807139f0abc4d0eb3577f71ce994f4f Mon Sep 17 00:00:00 2001 From: Jesse Attas Date: Thu, 19 Sep 2024 16:25:46 -0500 Subject: [PATCH 02/11] Restore / update comment about unbound-method --- packages/eslint-config-angular/requiring-type-checking.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/eslint-config-angular/requiring-type-checking.js b/packages/eslint-config-angular/requiring-type-checking.js index 755c174..a36883e 100644 --- a/packages/eslint-config-angular/requiring-type-checking.js +++ b/packages/eslint-config-angular/requiring-type-checking.js @@ -7,6 +7,10 @@ module.exports = { { files: ['*.spec.ts'], rules: { + /* + Spies used by Angular application tests result in a significant number of + unbound methods so this rule is disabled for test specs in Angular projects. + */ '@typescript-eslint/unbound-method': 'off', } }, From d3cc2f75a7aa76ca85cb35ee52274418bd9aac4f Mon Sep 17 00:00:00 2001 From: Jesse Attas Date: Thu, 19 Sep 2024 16:49:57 -0500 Subject: [PATCH 03/11] Enable require-await --- packages/eslint-config-javascript/index.js | 2 ++ .../lib/extensions-requiring-type-checking.js | 4 ++-- tests/angular/custom-ignore-attributes/index.spec.ts | 4 ++-- tests/angular/index.spec.ts | 4 ++-- tests/javascript/index.js | 4 ++++ tests/typescript-requiring-type-checking/index.ts | 6 ++++++ 6 files changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/eslint-config-javascript/index.js b/packages/eslint-config-javascript/index.js index d82ce41..99f899d 100644 --- a/packages/eslint-config-javascript/index.js +++ b/packages/eslint-config-javascript/index.js @@ -424,6 +424,8 @@ module.exports = { */ 'prefer-regex-literals': 'error', + 'require-await': 'error', + /* This configuration already supports the JSDoc syntax. Add additional syntax as line or block exceptions or markers when necessary. diff --git a/packages/eslint-config-typescript/lib/extensions-requiring-type-checking.js b/packages/eslint-config-typescript/lib/extensions-requiring-type-checking.js index 511117a..c4ddc48 100644 --- a/packages/eslint-config-typescript/lib/extensions-requiring-type-checking.js +++ b/packages/eslint-config-typescript/lib/extensions-requiring-type-checking.js @@ -12,9 +12,9 @@ module.exports = { 'no-throw-literal': 'off', '@typescript-eslint/no-throw-literal': 'error', - // Defined by Airbnb + // Defined by NI 'require-await': 'off', - '@typescript-eslint/require-await': 'off', + '@typescript-eslint/require-await': 'error', // Defined by Airbnb 'no-return-await': 'off', diff --git a/tests/angular/custom-ignore-attributes/index.spec.ts b/tests/angular/custom-ignore-attributes/index.spec.ts index cde10b1..00a0558 100644 --- a/tests/angular/custom-ignore-attributes/index.spec.ts +++ b/tests/angular/custom-ignore-attributes/index.spec.ts @@ -18,7 +18,7 @@ describe('MyComponent', () => { it('should have a div', async () => { await fixture.whenStable(); - expect(hostComponent.div).toBeDefined(); - expect(hostComponent.myMethod).not.toHaveBeenCalled(); + await expect(hostComponent.div).toBeDefined(); + await expect(hostComponent.myMethod).not.toHaveBeenCalled(); }); }); diff --git a/tests/angular/index.spec.ts b/tests/angular/index.spec.ts index ceb1fb5..7553a59 100644 --- a/tests/angular/index.spec.ts +++ b/tests/angular/index.spec.ts @@ -18,7 +18,7 @@ describe('MyComponent', () => { it('should have a div', async () => { await fixture.whenStable(); - expect(hostComponent.div).toBeDefined(); - expect(hostComponent.myMethod).not.toHaveBeenCalled(); + await expect(hostComponent.div).toBeDefined(); + await expect(hostComponent.myMethod).not.toHaveBeenCalled(); }); }); diff --git a/tests/javascript/index.js b/tests/javascript/index.js index 6f2392f..70fd742 100644 --- a/tests/javascript/index.js +++ b/tests/javascript/index.js @@ -12,4 +12,8 @@ export class NI { makeAwesomer() { this._awesomeLevel += 1; } + + async asyncAwesomeness() { + await new Promise(); + } } diff --git a/tests/typescript-requiring-type-checking/index.ts b/tests/typescript-requiring-type-checking/index.ts index bb4127e..53f809f 100644 --- a/tests/typescript-requiring-type-checking/index.ts +++ b/tests/typescript-requiring-type-checking/index.ts @@ -10,4 +10,10 @@ export class NI { public makeAwesomer(): void { this._awesomeLevel += 1; } + + public async asyncAwesomeness(): Promise { + await new Promise(resolve => { + resolve(true); + }); + } } From 34674aee1eda8377a91b94c55440ce7ef0261e3b Mon Sep 17 00:00:00 2001 From: Jesse Attas Date: Thu, 19 Sep 2024 16:59:09 -0500 Subject: [PATCH 04/11] Turn off deprecated JS rule no-return-await --- packages/eslint-config-javascript/index.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/eslint-config-javascript/index.js b/packages/eslint-config-javascript/index.js index 99f899d..fcaf5a5 100644 --- a/packages/eslint-config-javascript/index.js +++ b/packages/eslint-config-javascript/index.js @@ -325,6 +325,11 @@ module.exports = { selector: 'CallExpression[callee.object.name=\'console\'][callee.property.name=/^(debug|info|time|timeEnd|trace)$/]', message: 'Unexpected property on console object was called' }], + /* + This rule is deprecated since ESLint 8.46.0 because returning an awaited value no longer generates an extra microtask. + https://eslint.org/docs/latest/rules/no-return-await + */ + 'no-return-await': 'off', /* Disallow returning values from setters. From 6cf27051a9eaa47e6c78af6264399e4518f2996e Mon Sep 17 00:00:00 2001 From: Jesse Attas Date: Thu, 19 Sep 2024 17:23:15 -0500 Subject: [PATCH 05/11] Change files --- ...onfig-angular-225ce3da-b49d-4df7-ace0-b99abb8fbcb0.json | 7 +++++++ ...ig-javascript-4569f2fb-83ed-4863-9f95-96f907ede343.json | 7 +++++++ ...ig-typescript-32797652-8b30-47bc-b259-dfe598d5a4e3.json | 7 +++++++ 3 files changed, 21 insertions(+) create mode 100644 change/@ni-eslint-config-angular-225ce3da-b49d-4df7-ace0-b99abb8fbcb0.json create mode 100644 change/@ni-eslint-config-javascript-4569f2fb-83ed-4863-9f95-96f907ede343.json create mode 100644 change/@ni-eslint-config-typescript-32797652-8b30-47bc-b259-dfe598d5a4e3.json diff --git a/change/@ni-eslint-config-angular-225ce3da-b49d-4df7-ace0-b99abb8fbcb0.json b/change/@ni-eslint-config-angular-225ce3da-b49d-4df7-ace0-b99abb8fbcb0.json new file mode 100644 index 0000000..225dad5 --- /dev/null +++ b/change/@ni-eslint-config-angular-225ce3da-b49d-4df7-ace0-b99abb8fbcb0.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Rule changes for asynchronous code:\\n 1. Functions marked `async` must now include an `await`. The rule `'@typescript-eslint/require-await'` changed from `'off'` to `'error'`.\\n 2. Test code must now correctly handle promises. The rule `'@typescript-eslint/no-floating-promises'` changed from `'off'` to `'error'`.", + "packageName": "@ni/eslint-config-angular", + "email": "jattasNI@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/change/@ni-eslint-config-javascript-4569f2fb-83ed-4863-9f95-96f907ede343.json b/change/@ni-eslint-config-javascript-4569f2fb-83ed-4863-9f95-96f907ede343.json new file mode 100644 index 0000000..a13eb3a --- /dev/null +++ b/change/@ni-eslint-config-javascript-4569f2fb-83ed-4863-9f95-96f907ede343.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Rule changes for asynchronous code:\\n 1. Functions marked `async` must now include an `await`. The rule `'require-await'` changed from `'off'` to `'error'`.\\n 2. It is now acceptable for a function to return an awaited promise. The rule `'no-return-await'` changed from `'error'` to `'off'`.", + "packageName": "@ni/eslint-config-javascript", + "email": "jattasNI@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/change/@ni-eslint-config-typescript-32797652-8b30-47bc-b259-dfe598d5a4e3.json b/change/@ni-eslint-config-typescript-32797652-8b30-47bc-b259-dfe598d5a4e3.json new file mode 100644 index 0000000..31f3957 --- /dev/null +++ b/change/@ni-eslint-config-typescript-32797652-8b30-47bc-b259-dfe598d5a4e3.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Rule changes for asynchronous code:\\n 1. Functions marked `async` must now include an `await`. The rule `'@typescript-eslint/require-await'` changed from `'off'` to `'error'`.", + "packageName": "@ni/eslint-config-typescript", + "email": "jattasNI@users.noreply.github.com", + "dependentChangeType": "patch" +} From 0d2d43b8a68ff968ae085e3c627fda4e94cb960b Mon Sep 17 00:00:00 2001 From: Jesse Attas Date: Fri, 20 Sep 2024 11:53:01 -0500 Subject: [PATCH 06/11] Fix jasmine types --- package-lock.json | 14 +++----------- .../angular/custom-ignore-attributes/index.spec.ts | 4 ++-- tests/angular/index.spec.ts | 4 ++-- tests/angular/package.json | 4 ++-- tests/playwright-requiring-type-checking/index.ts | 2 +- 5 files changed, 10 insertions(+), 18 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8e32822..dead5e0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1661,18 +1661,10 @@ }, "node_modules/@types/jasmine": { "version": "5.1.4", - "resolved": "https://registry.npmjs.org/@types/jasmine/-/jasmine-5.1.4.tgz", + "resolved": "https://pull.artifacts.ni.com/artifactory/api/npm/rnd-npm-ci/@types/jasmine/-/jasmine-5.1.4.tgz", "integrity": "sha512-px7OMFO/ncXxixDe1zR13V1iycqWae0MxTaw62RpFlksUi5QuNWgQJFkTQjIOvrmutJbI7Fp2Y2N1F6D2R4G6w==", - "dev": true - }, - "node_modules/@types/jasminewd2": { - "version": "2.0.13", - "resolved": "https://registry.npmjs.org/@types/jasminewd2/-/jasminewd2-2.0.13.tgz", - "integrity": "sha512-aJ3wj8tXMpBrzQ5ghIaqMisD8C3FIrcO6sDKHqFbuqAsI7yOxj0fA7MrRCPLZHIVUjERIwsMmGn/vB0UQ9u0Hg==", "dev": true, - "dependencies": { - "@types/jasmine": "*" - } + "license": "MIT" }, "node_modules/@types/json-schema": { "version": "7.0.15", @@ -7920,7 +7912,7 @@ "@angular/core": "^17.3.12" }, "devDependencies": { - "@types/jasminewd2": "^2.0.13" + "@types/jasmine": "^5.1.4" }, "peerDependencies": { "@ni/eslint-config-angular": "*" diff --git a/tests/angular/custom-ignore-attributes/index.spec.ts b/tests/angular/custom-ignore-attributes/index.spec.ts index 00a0558..cde10b1 100644 --- a/tests/angular/custom-ignore-attributes/index.spec.ts +++ b/tests/angular/custom-ignore-attributes/index.spec.ts @@ -18,7 +18,7 @@ describe('MyComponent', () => { it('should have a div', async () => { await fixture.whenStable(); - await expect(hostComponent.div).toBeDefined(); - await expect(hostComponent.myMethod).not.toHaveBeenCalled(); + expect(hostComponent.div).toBeDefined(); + expect(hostComponent.myMethod).not.toHaveBeenCalled(); }); }); diff --git a/tests/angular/index.spec.ts b/tests/angular/index.spec.ts index 7553a59..ceb1fb5 100644 --- a/tests/angular/index.spec.ts +++ b/tests/angular/index.spec.ts @@ -18,7 +18,7 @@ describe('MyComponent', () => { it('should have a div', async () => { await fixture.whenStable(); - await expect(hostComponent.div).toBeDefined(); - await expect(hostComponent.myMethod).not.toHaveBeenCalled(); + expect(hostComponent.div).toBeDefined(); + expect(hostComponent.myMethod).not.toHaveBeenCalled(); }); }); diff --git a/tests/angular/package.json b/tests/angular/package.json index 2c9bc5f..90b16fb 100644 --- a/tests/angular/package.json +++ b/tests/angular/package.json @@ -13,6 +13,6 @@ "@angular/core": "^17.3.12" }, "devDependencies": { - "@types/jasminewd2": "^2.0.13" + "@types/jasmine": "^5.1.4" } -} \ No newline at end of file +} diff --git a/tests/playwright-requiring-type-checking/index.ts b/tests/playwright-requiring-type-checking/index.ts index c71e84e..944e258 100644 --- a/tests/playwright-requiring-type-checking/index.ts +++ b/tests/playwright-requiring-type-checking/index.ts @@ -1,6 +1,6 @@ import { test, expect } from '@playwright/test'; -describe('AssetDisplayPropertiesSidenavWrapperComponent', () => { +test.describe('AssetDisplayPropertiesSidenavWrapperComponent', () => { test('has title', async ({ page }) => { await page.goto('https://playwright.dev/'); From bf854fc2801dae2f402f32156f56405b8a80dfb2 Mon Sep 17 00:00:00 2001 From: Jesse Attas Date: Fri, 20 Sep 2024 12:04:02 -0500 Subject: [PATCH 07/11] Add npmrc and fix package-lock jfrog URL --- .gitignore | 1 - .npmrc | 1 + package-lock.json | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) create mode 100644 .npmrc diff --git a/.gitignore b/.gitignore index c38d31e..4e5a488 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,5 @@ node_modules .DS_Store *.tgz .vscode -.npmrc ./rules.txt ./rules-diff.txt \ No newline at end of file diff --git a/.npmrc b/.npmrc new file mode 100644 index 0000000..214c29d --- /dev/null +++ b/.npmrc @@ -0,0 +1 @@ +registry=https://registry.npmjs.org/ diff --git a/package-lock.json b/package-lock.json index dead5e0..7c88658 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1661,7 +1661,7 @@ }, "node_modules/@types/jasmine": { "version": "5.1.4", - "resolved": "https://pull.artifacts.ni.com/artifactory/api/npm/rnd-npm-ci/@types/jasmine/-/jasmine-5.1.4.tgz", + "resolved": "https://registry.npmjs.org/@types/jasmine/-/jasmine-5.1.4.tgz", "integrity": "sha512-px7OMFO/ncXxixDe1zR13V1iycqWae0MxTaw62RpFlksUi5QuNWgQJFkTQjIOvrmutJbI7Fp2Y2N1F6D2R4G6w==", "dev": true, "license": "MIT" From 504f928065d78f3df215d30493849312d47a3b2b Mon Sep 17 00:00:00 2001 From: Jesse Attas Date: Fri, 20 Sep 2024 16:01:09 -0500 Subject: [PATCH 08/11] Promise.resolve Co-authored-by: Milan Raj --- tests/typescript-requiring-type-checking/index.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/typescript-requiring-type-checking/index.ts b/tests/typescript-requiring-type-checking/index.ts index 53f809f..8360128 100644 --- a/tests/typescript-requiring-type-checking/index.ts +++ b/tests/typescript-requiring-type-checking/index.ts @@ -12,8 +12,6 @@ export class NI { } public async asyncAwesomeness(): Promise { - await new Promise(resolve => { - resolve(true); - }); + await Promise.resolve(true); } } From aeaafbcc6b938bb05747fb135a02bd85d8f43f37 Mon Sep 17 00:00:00 2001 From: Jesse Attas Date: Fri, 20 Sep 2024 16:01:18 -0500 Subject: [PATCH 09/11] Promise.resolve Co-authored-by: Milan Raj --- tests/javascript/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/javascript/index.js b/tests/javascript/index.js index 70fd742..6c55621 100644 --- a/tests/javascript/index.js +++ b/tests/javascript/index.js @@ -14,6 +14,6 @@ export class NI { } async asyncAwesomeness() { - await new Promise(); + await Promise.resolve(42); } } From d293fa31cd459f775d788f95a66bbc89282333fa Mon Sep 17 00:00:00 2001 From: Jesse Attas Date: Fri, 20 Sep 2024 16:02:50 -0500 Subject: [PATCH 10/11] Comment in config file for require-await --- packages/eslint-config-javascript/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/eslint-config-javascript/index.js b/packages/eslint-config-javascript/index.js index fcaf5a5..dc78110 100644 --- a/packages/eslint-config-javascript/index.js +++ b/packages/eslint-config-javascript/index.js @@ -429,6 +429,9 @@ module.exports = { */ 'prefer-regex-literals': 'error', + /* + Asynchronous functions that don’t use await might not need to be asynchronous functions and could be the unintentional result of refactoring. + */ 'require-await': 'error', /* From 003b132331ff0282b1988bf92d89550bf1e7367e Mon Sep 17 00:00:00 2001 From: Jesse Attas Date: Thu, 26 Sep 2024 12:05:20 -0500 Subject: [PATCH 11/11] return-await always --- .../lib/extensions-requiring-type-checking.js | 2 +- tests/typescript-requiring-type-checking/index.ts | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/eslint-config-typescript/lib/extensions-requiring-type-checking.js b/packages/eslint-config-typescript/lib/extensions-requiring-type-checking.js index c4ddc48..1386d2c 100644 --- a/packages/eslint-config-typescript/lib/extensions-requiring-type-checking.js +++ b/packages/eslint-config-typescript/lib/extensions-requiring-type-checking.js @@ -18,6 +18,6 @@ module.exports = { // Defined by Airbnb 'no-return-await': 'off', - '@typescript-eslint/return-await': 'error', + '@typescript-eslint/return-await': ['error', 'always'], } }; diff --git a/tests/typescript-requiring-type-checking/index.ts b/tests/typescript-requiring-type-checking/index.ts index 8360128..e8fea41 100644 --- a/tests/typescript-requiring-type-checking/index.ts +++ b/tests/typescript-requiring-type-checking/index.ts @@ -11,7 +11,9 @@ export class NI { this._awesomeLevel += 1; } - public async asyncAwesomeness(): Promise { - await Promise.resolve(true); + public async slowAdd(a: number, b: number, timeMs: number): Promise { + return await new Promise(resolve => { + setTimeout(() => resolve(a + b), timeMs); + }); } }