Skip to content

Commit

Permalink
Optimize loop in lint() (#245)
Browse files Browse the repository at this point in the history
* Return null instead of object if no failing rules found

* Make single pass through rules

* Add note to CHANGELOG for #245
  • Loading branch information
TrevorBurnham authored and amilajack committed Jul 1, 2019
1 parent e0e8e2f commit 5e4fc13
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 35 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## v3.3.0
### Performance
- Optimize core loop to run ~50% faster ([https://github.com/amilajack/eslint-plugin-compat/pull/245](https://github.com/amilajack/eslint-plugin-compat/pull/245))

## v3.2.0
### Added
- Support for `eslint@6`
Expand Down
38 changes: 16 additions & 22 deletions src/Lint.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// @flow
import { rules } from './providers';
import type { Node, ESLintNode, Targets, isValidObject } from './LintTypes';
import type { Node, ESLintNode, Targets, lintResultObject } from './LintTypes';

export function generateErrorName(_node: Node): string {
if (_node.name) return _node.name;
Expand All @@ -18,35 +18,29 @@ export default function Lint(
eslintNode: ESLintNode,
targets: Targets = ['chrome', 'firefox', 'safari', 'edge'],
polyfills: Set<string>
): isValidObject {
): ?lintResultObject {
// Find the corresponding rules for a eslintNode by it's astNodeType
const failingRule = rules
.filter(
(rule: Node): boolean =>
rule.astNodeType === eslintNode.type &&
// v2 allowed users to select polyfills based off their caniuseId. This is
// no longer supported. Keeping this here to avoid breaking changes.
!polyfills.has(rule.id) &&
// Check if polyfill is provided (ex. `Promise.all`)
!polyfills.has(rule.protoChainId) &&
// Check if entire API is polyfilled (ex. `Promise`)
!polyfills.has(rule.protoChain[0])
)
// Find the first failing rule
.find((rule: Node): boolean => !rule.isValid(rule, eslintNode, targets));
const failingRule = rules.find(
(rule: Node): boolean =>
rule.astNodeType === eslintNode.type &&
// Check that the rule fails for this node (unless there's a polyfill)
!rule.isValid(rule, eslintNode, targets) &&
// v2 allowed users to select polyfills based off their caniuseId. This is
// no longer supported. Keeping this here to avoid breaking changes.
!polyfills.has(rule.id) &&
// Check if polyfill is provided (ex. `Promise.all`)
!polyfills.has(rule.protoChainId) &&
// Check if entire API is polyfilled (ex. `Promise`)
!polyfills.has(rule.protoChain[0])
);

return failingRule
? {
rule: failingRule,
isValid: false,
unsupportedTargets: failingRule.getUnsupportedTargets(
failingRule,
targets
)
}
: {
rule: {},
unsupportedTargets: [],
isValid: true
};
: null;
}
3 changes: 1 addition & 2 deletions src/LintTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ export type Node = {
) => boolean
};

export type isValidObject = {
export type lintResultObject = {
rule: Node,
isValid: boolean,
unsupportedTargets: Array<string>
};
23 changes: 12 additions & 11 deletions src/rules/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,22 +60,23 @@ export default {
const errors = [];

function lint(node: ESLintNode) {
const { isValid, rule, unsupportedTargets } = Lint(
const lintResult = Lint(
node,
browserslistTargets,
new Set(context.settings.polyfills || [])
);

if (!isValid) {
errors.push({
node,
message: [
generateErrorName(rule),
'is not supported in',
unsupportedTargets.join(', ')
].join(' ')
});
}
if (lintResult == null) return;

const { rule, unsupportedTargets } = lintResult;
errors.push({
node,
message: [
generateErrorName(rule),
'is not supported in',
unsupportedTargets.join(', ')
].join(' ')
});
}

const identifiers = new Set();
Expand Down

0 comments on commit 5e4fc13

Please sign in to comment.