amilajack
Feature detection change stops all compat linting of the content of any if statements
I just recently noticed that when comparing 3.7 and 3.8 it appears that compat linting has been broken for the contents of IfStatements.
It appears that the goal of feat: allow feature detection of APIs #327 is that when a IfStatement is used for feature detection that the IfStatement's condition or its block (and any further nested blocks) will not report compatibility errors for that detected feature.
E.g.
if (fetch) {
fetch()
}
But the isInsideIfStatement function that is used only checks that node has an ancestor that is an IfStatement:
function isInsideIfStatement(context) {
return context.getAncestors().some((ancestor) => {
return ancestor.type === "IfStatement";
});
}
This is effectively turning off compat linting for anything which is inside any IfStatement, whether it is used for detecting the feature or otherwise.
if (true) {
fetch(); // doesn't report compat error
}
fetch(); // reports compat error
It seems like the desired result would be that the isInsideIfStatement should be checking that the rule is inside an if statement (no matter how nested) that has tested for that particular feature.
if (Array.prototype.flat) {
new Array.flat(); // wouldn't report compat error
fetch(); // would report compat error
if (fetch) {
new Array.flat(); // wouldn't report compat error
fetch(); // wouldn't report compat error
}
}
@amilajack Could I get your insights into this?
This effectively make the plugin useless? It's pretty trivial to break the rule. @amilajack
I agree, the current isInsideIfStatement isn't very effective. properly implement a good solution for this is non-trivial. I haven't gotten around to it just yet but feel free to take a stab at it
