Skip to content

Commit

Permalink
Enable require-await rule (#833)
Browse files Browse the repository at this point in the history
# 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](#831 (comment)).

This is disabled in the ni eslint rules due to airbnb's configuration
which I think [is not well
justified](airbnb/javascript@0b3b5a1#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.
  • Loading branch information
rajsite committed Nov 29, 2022
1 parent a284cb2 commit 3ff74ec
Show file tree
Hide file tree
Showing 13 changed files with 58 additions and 11 deletions.
11 changes: 9 additions & 2 deletions angular-workspace/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down Expand Up @@ -58,5 +61,9 @@ module.exports = {
extends: [
'@ni/eslint-config-javascript'
],
rules: {
// Enabled to prevent accidental usage of async-await
'require-await': 'error'
}
}]
};
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "Changes from updated require-await lint rule configuration",
"packageName": "@ni/nimble-blazor",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "Changed from updated require-await lint rule configuration",
"packageName": "@ni/nimble-components",
"email": "[email protected]",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "Changes from updated require-await lint rule configuration",
"packageName": "@ni/nimble-tokens",
"email": "[email protected]",
"dependentChangeType": "none"
}
5 changes: 4 additions & 1 deletion packages/nimble-blazor/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
}
]
Expand Down
4 changes: 4 additions & 0 deletions packages/nimble-components/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ module.exports = {
'node_modules',
'dist'
],
rules: {
// Enabled to prevent accidental usage of async-await
'require-await': 'error'
},
overrides: [
{
files: ['.storybook/**'],
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/.storybook/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module.exports = {
features: {
previewCsfV3: true
},
webpackFinal: async config => {
webpackFinal: config => {
config.module.rules.push({
test: /\.ts$/,
use: [
Expand Down
4 changes: 3 additions & 1 deletion packages/nimble-components/build/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion packages/nimble-components/src/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ module.exports = {
}
]
}
]
],

// Enabled to prevent accidental usage of async-await
'@typescript-eslint/require-await': 'error'
},
ignorePatterns: ['.eslintrc.js'],
overrides: [
Expand Down
6 changes: 3 additions & 3 deletions packages/nimble-components/src/drawer/tests/drawer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,21 +212,21 @@ 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();
void element.show();
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
5 changes: 4 additions & 1 deletion packages/nimble-tokens/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
}
]
Expand Down
4 changes: 4 additions & 0 deletions packages/site/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,8 @@ module.exports = {
'node_modules',
'dist'
],
rules: {
// Enabled to prevent accidental usage of async-await
'require-await': 'error'
}
};

0 comments on commit 3ff74ec

Please sign in to comment.