From 3ff74ec1486e824339f81b27729aa052b083cd5e Mon Sep 17 00:00:00 2001 From: Milan Raj Date: Tue, 29 Nov 2022 17:41:57 -0600 Subject: [PATCH] Enable require-await rule (#833) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Pull Request ## ๐Ÿคจ Rationale Enables the eslint [require-await](https://eslint.org/docs/latest/rules/require-await) rule after noticing some spots where `async` was [used unexpectedly](https://github.com/ni/nimble/pull/831#discussion_r1029679411). This is disabled in the ni eslint rules due to airbnb's configuration which I think [is not well justified](https://github.com/airbnb/javascript/commit/0b3b5a178fb32bfc60096738369e069c8cfcfa91#r35614411). At least in nimble we should prefer async-await syntax AND prefer to not mix async and promise syntax. Enabling this rule helps prevent mixing syntaxes and so far with enabling it helped prevent introducing unnecessary / unexpected async functions. ## ๐Ÿ‘ฉโ€๐Ÿ’ป Implementation Enabled `require-await` or `@typescript-eslint/require-await` in each `.eslintrs.jc` file. The rule found additional places in source where the use of `async` was unnecessary. ## ๐Ÿงช Testing Rely on CI. ## โœ… Checklist - [x] I have updated the project documentation to reflect my changes or determined no changes are needed. --- angular-workspace/.eslintrc.js | 11 +++++++++-- ...e-blazor-91358d04-10a3-450e-995b-8970f1128006.json | 7 +++++++ ...mponents-3b4e5324-510c-4cff-a98f-cf216606e551.json | 7 +++++++ ...e-tokens-c57f19cd-796f-48aa-961a-7fb8be603668.json | 7 +++++++ packages/nimble-blazor/.eslintrc.js | 5 ++++- packages/nimble-components/.eslintrc.js | 4 ++++ packages/nimble-components/.storybook/main.js | 2 +- packages/nimble-components/build/.eslintrc.js | 4 +++- packages/nimble-components/src/.eslintrc.js | 5 ++++- .../nimble-components/src/drawer/tests/drawer.spec.ts | 6 +++--- .../src/tree-view/tests/tree.spec.ts | 2 +- packages/nimble-tokens/.eslintrc.js | 5 ++++- packages/site/.eslintrc.js | 4 ++++ 13 files changed, 58 insertions(+), 11 deletions(-) create mode 100644 change/@ni-nimble-blazor-91358d04-10a3-450e-995b-8970f1128006.json create mode 100644 change/@ni-nimble-components-3b4e5324-510c-4cff-a98f-cf216606e551.json create mode 100644 change/@ni-nimble-tokens-c57f19cd-796f-48aa-961a-7fb8be603668.json diff --git a/angular-workspace/.eslintrc.js b/angular-workspace/.eslintrc.js index 79ff10479c..1e90b76b37 100644 --- a/angular-workspace/.eslintrc.js +++ b/angular-workspace/.eslintrc.js @@ -24,7 +24,10 @@ module.exports = { allowNullableBoolean: true, allowNullableString: true, allowNullableNumber: false - }] + }], + + // Enabled to prevent accidental usage of async-await + '@typescript-eslint/require-await': 'error' } }, { files: ['*.spec.ts'], @@ -58,5 +61,9 @@ module.exports = { extends: [ '@ni/eslint-config-javascript' ], + rules: { + // Enabled to prevent accidental usage of async-await + 'require-await': 'error' + } }] -}; \ No newline at end of file +}; diff --git a/change/@ni-nimble-blazor-91358d04-10a3-450e-995b-8970f1128006.json b/change/@ni-nimble-blazor-91358d04-10a3-450e-995b-8970f1128006.json new file mode 100644 index 0000000000..46619c1005 --- /dev/null +++ b/change/@ni-nimble-blazor-91358d04-10a3-450e-995b-8970f1128006.json @@ -0,0 +1,7 @@ +{ + "type": "none", + "comment": "Changes from updated require-await lint rule configuration", + "packageName": "@ni/nimble-blazor", + "email": "rajsite@users.noreply.github.com", + "dependentChangeType": "none" +} diff --git a/change/@ni-nimble-components-3b4e5324-510c-4cff-a98f-cf216606e551.json b/change/@ni-nimble-components-3b4e5324-510c-4cff-a98f-cf216606e551.json new file mode 100644 index 0000000000..4cd0d9d63e --- /dev/null +++ b/change/@ni-nimble-components-3b4e5324-510c-4cff-a98f-cf216606e551.json @@ -0,0 +1,7 @@ +{ + "type": "none", + "comment": "Changed from updated require-await lint rule configuration", + "packageName": "@ni/nimble-components", + "email": "rajsite@users.noreply.github.com", + "dependentChangeType": "none" +} diff --git a/change/@ni-nimble-tokens-c57f19cd-796f-48aa-961a-7fb8be603668.json b/change/@ni-nimble-tokens-c57f19cd-796f-48aa-961a-7fb8be603668.json new file mode 100644 index 0000000000..7d4fa9eb11 --- /dev/null +++ b/change/@ni-nimble-tokens-c57f19cd-796f-48aa-961a-7fb8be603668.json @@ -0,0 +1,7 @@ +{ + "type": "none", + "comment": "Changes from updated require-await lint rule configuration", + "packageName": "@ni/nimble-tokens", + "email": "rajsite@users.noreply.github.com", + "dependentChangeType": "none" +} diff --git a/packages/nimble-blazor/.eslintrc.js b/packages/nimble-blazor/.eslintrc.js index a5d54a7cc7..89b4b203a8 100644 --- a/packages/nimble-blazor/.eslintrc.js +++ b/packages/nimble-blazor/.eslintrc.js @@ -16,7 +16,10 @@ module.exports = { 'no-console': 'off', // Rollup config files use default exports - 'import/no-default-export': 'off' + 'import/no-default-export': 'off', + + // Enabled to prevent accidental usage of async-await + 'require-await': 'error' } } ] diff --git a/packages/nimble-components/.eslintrc.js b/packages/nimble-components/.eslintrc.js index 0b8c9675ab..c6fad82899 100644 --- a/packages/nimble-components/.eslintrc.js +++ b/packages/nimble-components/.eslintrc.js @@ -10,6 +10,10 @@ module.exports = { 'node_modules', 'dist' ], + rules: { + // Enabled to prevent accidental usage of async-await + 'require-await': 'error' + }, overrides: [ { files: ['.storybook/**'], diff --git a/packages/nimble-components/.storybook/main.js b/packages/nimble-components/.storybook/main.js index 4b13842355..3027fff13a 100644 --- a/packages/nimble-components/.storybook/main.js +++ b/packages/nimble-components/.storybook/main.js @@ -19,7 +19,7 @@ module.exports = { features: { previewCsfV3: true }, - webpackFinal: async config => { + webpackFinal: config => { config.module.rules.push({ test: /\.ts$/, use: [ diff --git a/packages/nimble-components/build/.eslintrc.js b/packages/nimble-components/build/.eslintrc.js index adbb84b519..a8f465285a 100644 --- a/packages/nimble-components/build/.eslintrc.js +++ b/packages/nimble-components/build/.eslintrc.js @@ -10,7 +10,9 @@ module.exports = { // Build scripts should give verbose logging 'no-console': 'off', // Rollup config files use default exports - 'import/no-default-export': 'off' + 'import/no-default-export': 'off', + // Enabled to prevent accidental usage of async-await + 'require-await': 'error' }, parserOptions: { ecmaVersion: 2020, diff --git a/packages/nimble-components/src/.eslintrc.js b/packages/nimble-components/src/.eslintrc.js index e5610d5b80..f1f62e97e9 100644 --- a/packages/nimble-components/src/.eslintrc.js +++ b/packages/nimble-components/src/.eslintrc.js @@ -57,7 +57,10 @@ module.exports = { } ] } - ] + ], + + // Enabled to prevent accidental usage of async-await + '@typescript-eslint/require-await': 'error' }, ignorePatterns: ['.eslintrc.js'], overrides: [ diff --git a/packages/nimble-components/src/drawer/tests/drawer.spec.ts b/packages/nimble-components/src/drawer/tests/drawer.spec.ts index 11164ee048..e570c3d156 100644 --- a/packages/nimble-components/src/drawer/tests/drawer.spec.ts +++ b/packages/nimble-components/src/drawer/tests/drawer.spec.ts @@ -212,13 +212,13 @@ describe('Drawer', () => { expect(afterDrawerCloseActiveElement).toBe(button2); }); - it('focuses the first button on the drawer when it opens', async () => { + it('focuses the first button on the drawer when it opens', () => { const okButton = document.getElementById('ok')!; void element.show(); expect(document.activeElement).toBe(okButton); }); - it('focuses the button with autofocus when the drawer opens', async () => { + it('focuses the button with autofocus when the drawer opens', () => { const cancelButton = document.getElementById('cancel')!; cancelButton.setAttribute('autofocus', ''); DOM.processUpdates(); @@ -226,7 +226,7 @@ describe('Drawer', () => { expect(document.activeElement).toBe(cancelButton); }); - it('supports opening multiple drawers on top of each other', async () => { + it('supports opening multiple drawers on top of each other', () => { const secondDrawer = document.createElement('nimble-drawer'); const secondDrawerButton = document.createElement('nimble-button'); secondDrawer.append(secondDrawerButton); diff --git a/packages/nimble-components/src/tree-view/tests/tree.spec.ts b/packages/nimble-components/src/tree-view/tests/tree.spec.ts index f5ec947958..108a825582 100644 --- a/packages/nimble-components/src/tree-view/tests/tree.spec.ts +++ b/packages/nimble-components/src/tree-view/tests/tree.spec.ts @@ -87,7 +87,7 @@ describe('TreeView', () => { ); }); - it('root1 should have "group-selected" attribute set after initialization', async () => { + it('root1 should have "group-selected" attribute set after initialization', () => { expect(model.root1.hasAttribute('group-selected')).toBe(true); expect(model.root2.hasAttribute('group-selected')).toBe(false); }); diff --git a/packages/nimble-tokens/.eslintrc.js b/packages/nimble-tokens/.eslintrc.js index 11fb254459..4b028867de 100644 --- a/packages/nimble-tokens/.eslintrc.js +++ b/packages/nimble-tokens/.eslintrc.js @@ -13,7 +13,10 @@ module.exports = { 'import/no-extraneous-dependencies': ['error', { devDependencies: true }], // Okay to use console.log in build scripts - 'no-console': 'off' + 'no-console': 'off', + + // Enabled to prevent accidental usage of async-await + 'require-await': 'error' } } ] diff --git a/packages/site/.eslintrc.js b/packages/site/.eslintrc.js index b7a6464615..0931981b48 100644 --- a/packages/site/.eslintrc.js +++ b/packages/site/.eslintrc.js @@ -9,4 +9,8 @@ module.exports = { 'node_modules', 'dist' ], + rules: { + // Enabled to prevent accidental usage of async-await + 'require-await': 'error' + } };