Guard super call with typeof check

[anthony-unicare]

It's better (more explicit) to guard a super call with a typeof check rather than a "truthy" check. Here's a patch to do that.

diff --git a/node_modules/eslint-plugin-wc/lib/rules/guard-super-call.js b/node_modules/eslint-plugin-wc/lib/rules/guard-super-call.js
index 873ec0d..ad659e4
--- a/node_modules/eslint-plugin-wc/lib/rules/guard-super-call.js
+++ b/node_modules/eslint-plugin-wc/lib/rules/guard-super-call.js
@@ -65,6 +65,19 @@ const rule = {
                 node.expression.type === 'CallExpression' &&
                 isSuperHook(node.expression.callee, hook));
         }
+        /**
+         * Determines if an if statement is a correct super hook guard
+         * @param {ESTree.IfStatement} node Node to test
+         * @param {string} hook hook to test
+         * @return {boolean}
+         */
+        function isCorrectSuperHookGuard(node, hook) {
+            return node.test.type === 'BinaryExpression' &&
+                node.test.left.operator === 'typeof' &&
+                isSuperHook(node.test.left.argument, hook) &&
+                node.test.right.type === 'Literal' &&
+                node.test.right.value === 'function';
+        }
         /**
          * Determines if a statement is an unguarded super hook
          * @param {ESTree.Statement} node Node to test
@@ -76,7 +89,7 @@ const rule = {
                 errNode = node;
                 return true;
             }
-            else if (node.type === 'IfStatement' && !isSuperHook(node.test, hook)) {
+            else if (node.type === 'IfStatement' && !isCorrectSuperHookGuard(node, hook)) {
                 return isUnguardedSuperHook(node.consequent, hook);
             }
             else if (node.type === 'BlockStatement' &&

[43081j]

it is stricter to check the type, probably a logical thing to do. however, there's already a lot of repos out there in the wild that just check the existence.

if we changed this behaviour, we'd cause a lot of headache in terms of people updating their code

given that many people are moving to typescript and needing this rule less over time, i think it makes sense to leave it as it is (even if its a looser check than it could be)

[anthony-unicare]

it is stricter to check the type, probably a logical thing to do. however, there's already a lot of repos out there in the wild that just check the existence.

if we changed this behaviour, we'd cause a lot of headache in terms of people updating their code

given that many people are moving to typescript and needing this rule less over time, i think it makes sense to leave it as it is (even if its a looser check than it could be)

Thank you for taking the time to look into my issue report and respond to it.

I understand your point about not wanting to break existing code. I can see two ways to incorporate the change I proposed without breaking anything:

1. add options to configure the rule and make the current behaviour the default; or
2. allow either style of check to pass the rule.

What do you think of these ideas?

[43081j]

we could possibly add an option to the rule, like requireTypeCheck or something along those lines

if it is enabled, require a typeof check, otherwise do the current logic

[anthony-unicare]

we could possibly add an option to the rule, like requireTypeCheck or something along those lines

if it is enabled, require a typeof check, otherwise do the current logic

That sounds perfect! 👍

[anthony-unicare]

@43081j, here’s a pull request: https://github.com/43081j/eslint-plugin-wc/pull/135.