mozilla / eslint-plugin-no-unsanitized

Custom ESLint rule to disallows unsafe innerHTML, outerHTML, insertAdjacentHTML and alike
Mozilla Public License 2.0
223 stars 34 forks source link

Improve performance of no-unsanitized/method #82

Closed Standard8 closed 6 years ago

Standard8 commented 6 years ago

Currently no-unsanitized/method is creating a new RuleContext, and combining an array of rules for pretty much each CallExpression.

I think this can be avoided to help performance.

Using's ESLint TIMING=1 environment variable and running against mozilla-central, I saw no-unsanitized/method go from being in the top-ten rules at about 2.6 seconds to dropping completely out of the list to less than 1.5 seconds (unfortunately they don't report more than the top-ten).

jonathanKingston commented 6 years ago

Hey, Thanks for the patch!

This seems fine for me so long as the scope is correctly changed per directory etc.

Standard8 commented 6 years ago
  • The rule checks are added to the scope and how often do those get checked? For example if I have a nested .eslint file does it get merged there?

This seems fine for me so long as the scope is correctly changed per directory etc.

Yes, the rule/context is recreated for each file (since we have things like context.getFilename() and context.getSource(), which will therefore construct a new RuleHelper.

The advantage this patch is giving us is that we won't be re-creating RuleHelper / combining the context rules every time we hit a CallExpression or an AssignmentExpression.

Standard8 commented 6 years ago

@jonathanKingston Did you see the updates I posted here?

Standard8 commented 6 years ago

@jonathanKingston @mozfreddyb ping?

jonathanKingston commented 6 years ago

@Standard8 sorry for the delay, thanks. We should probably do a version bump now.

mozfreddyb commented 6 years ago

just published v3.0.2 on npm