Repository navigation
Narrowing the type inside forEach doesn't follow the if check from the outside #56854
Description
🔎 Search Terms
type narrowing
🕗 Version & Regression Information
- I was unable to test this on prior versions because I was testing in the playground
⏯ Playground Link
💻 Code
const empty_rule = (sheet: CSSStyleSheet, class_name: string) => {
const index = sheet.cssRules.length;
sheet.insertRule(`.${class_name} { }`, index);
return sheet.cssRules[index];
};
const set_css = (sheet: CSSStyleSheet, class_name: string, style: Style) => {
const selector = `.${class_name}`;
let rule = [...sheet.cssRules].find(rule => {
return rule instanceof CSSStyleRule && rule.selectorText === selector;
});
if (!rule) {
rule = empty_rule(sheet, class_name);
}
if (rule instanceof CSSStyleRule) {
const x = rule.style;
Object.entries(style).forEach(([prop, value]) => {
// this throw an error
rule.style.setProperty(prop, value);
});
Object.entries(style).forEach(([prop, value]) => {
// this works
x.setProperty(prop, value);
});
}
};🙁 Actual behavior
You can't use a variable that was type narrowed inside forEach. You got an error:
Property 'style' does not exist on type 'CSSRule'.
🙂 Expected behavior
I expect rule.style to work the same inside map.
Additional information about the issue
It looks like inside forEach all typechecks are ignored. Another issue is that the rule can be undefined inside forEach but outside it's ok.
Activity
jcalz commentedon Dec 22, 2023
Another one for the #9998 pile.
fatcerberus commentedon Dec 22, 2023
Yep, and there’s even a whole issue template explaining not to post this exact issue and everything (“types not correct with or in callback”)…
jcubic commentedon Dec 23, 2023
I was looking into FAQ but I didn't find an issue like this. Where does it say to not post those types of issues?
jcalz commentedon Dec 23, 2023
It’s like the sixth button when you open a new issue… but if I were filing a bug report and didn’t already know about it I doubt I’d notice it. Ideally such a thing would be mentioned inside the bug report template itself, but 🤷♂️.
jcubic commentedon Dec 23, 2023
I would put it into the FAQ since the issue says to check the FAQ first before creating an issue.
fatcerberus commentedon Dec 23, 2023
The explanation in question:
https://github.com/microsoft/TypeScript/issues/new?assignees=&labels=Duplicate&projects=&template=types-not-correct-in-with-callback.md&title=
I’m honestly surprised it isn’t listed in the “Bugs That Aren’t Bugs” section of the FAQ, there’s an issue like this posted at least once a week. People hit this all the time.
Also relevant is #11498 which discusses a way this might be fixed for certain cases (like this one).
jcubic commentedon Dec 23, 2023
The reason why I and probably everyone else didn't see the question is because people read from top to bottom until they found a proper issue type. They don't read the rest of the issues just because they may have important information, who does that? I would definitely add this to the FAQ instead of adding a fake issue.
I would also get rid of those fake issues that answer the problem and put everything into FAQ. That is supposed to Be a place to read common bugs that are not bugs.
jcubic commentedon Dec 23, 2023
I just read the fake issue template and I don't really understand. How:
is the same as setTimeout which can get executed after delay and something else can modify the code. This code doesn't have any side effects. Those are built-in methods of JavaScript. the function is obviously a callback like in the issue, but shouldn't this work differently than async functions?
jcalz commentedon Dec 23, 2023
The type system cannot express that a callback is going to be called immediately, so it can’t tell the difference between those two examples. Please see #11498
fatcerberus commentedon Dec 23, 2023
I don't think you're expected to read all the templates (indeed, who does that?), just to recognize that "types not correct in/with callback1" covers your issue and then, when you click it, it gives you more context. Interestingly, people do pick that template sometimes... but for completely unrelated issues that then get erroneously tagged as duplicates 😅
I agree this should be in the FAQ though and I'm surprised it isn't.
Footnotes
I suspect a large part of the problem is that people don't intuitively consider the function passed to, e.g.,
.forEach, as being a "callback", as that term carries a lot of baggage relating to asynchronous operation. An equivalent FAQ entry would have to be very carefully worded or people will likely skim past it, too. ↩jcubic commentedon Dec 23, 2023
If there was an entry like that in the FAQ I would never create an issue. For me, the callback is always a function passed as an argument to another function.
MartinJohns commentedon Dec 26, 2023
🤚
RyanCavanaugh commentedon Jan 2, 2024
"First match wins" overload resolution remains a somewhat problematic design aspect of both TypeScript and TypeScript users, apparently 🙂
jcalz commentedon Jan 2, 2024
That also explains why I only think there's a "Website" button when I talk about that page in the abstract.