ni / javascript-styleguide

JavaScript and TypeScript Style Guide
MIT License
9 stars 9 forks source link

Disable no-await-in-loop for Playwright ruleset #123

Open jattasNI opened 1 year ago

jattasNI commented 1 year ago

Describe the issue

There are dozens of suppressions of no-await-in-loop in Skyline Playwright code. They don't typically include an explanation why, but it is presumably because they are performing a UI operation that must be serialized, so the rule is not making a useful suggestion.

    public async expandParametricMeasurementTree(itemsToExpand: string[]): Promise<void> {
        for (const item of itemsToExpand) {
            // eslint-disable-next-line no-await-in-loop
            await this.parameterTree.locator(`nimble-tree-item[title="${item}"]`).click();
        }
    }

As a counterpoint, there are some cases where code is calling expect on an awaited statement in a loop. These could possibly be rewritten to be more parallelized. But it would likely hurt readability and not significantly improve performance

        for (const data of initialProperties) {
            expect(await assetDetailsPage.getDetailsPanelSingleItem(initialProperties.indexOf(data))).toContain(data);
        }

        for (const data of displayPanelData) {
            expect(await assetDetailsPage.getDetailsPanelSingleValue(displayPanelData.indexOf(data))).toContain(data);
        }

I propose we disable it in the playwright rule set so we can remove these suppressions in codebases that adopt the ruleset.

jattasNI commented 1 year ago

We agreed to disable the rule for the Playwright ruleset. As part of this we could also tweak our app acceptance test eslint configs to make it more obvious that they target only the tests, not the server code (which is JS code).

We'll take no action for Jasmine code for now, though someday in future we may want separate configurations for Jasmine test code.