-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Add @ts-ignore-enable @ts-ignore-disable #40267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This PR add two directives
... <-- this is normal
// @ts-ignore-enable
XXX <-- this is ignored
... <-- this is ignored
// @ts-ignore-enable
XXX <-- this is ignored
// @ts-ignore-enable <-- you could remove this line.
YYY <-- this is ignored
// @ts-ignore-disable
ZZZ <-- this is normal
// @ts-ignore-disable <-- you could remove this line.
// @ts-ignore-enable
XXX <-- this is ignored
// @ts-expect-error <-- No error message. whatever next line has error or not.
YYY <-- this is ignored
// @ts-ignore-enable A
XXX <-- this is ignored
// @ts-ignore-enable B <-- you could remove this line.
YYY <-- this is ignored
// @ts-ignore-disable B
ZZZ <-- this is normal
// @ts-ignore-disable A <-- you could remove this line. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the reasons that @weswigham gave for not doing this are pretty good. I'm voting no unless we change our minds on the original bug.
Yep, I strongly agree. I give this PR because one comment is really interesting. #19573 (comment) And maybe this PR could let community try giving more persuasive reasons, rather than just asking for it again and again. |
I would like to reiterate my comment, that hasn't gotten a response from the team. |
See resolution in #40267 (comment)
|
@kumar303 Auh, not sure whether I get the point. But I download styled-componets and @type/styled-components and have following code import styled, { css } from 'styled-components'
const Button = styled.button`
${props =>
css`
background: palevioletred;
color: white;
`};
` I do not get any error(at least in my simple ts-config). Now make a little change to add error: const Button = styled.button`
${props =>
props.primary && // ts(2339) on text `primary`
css`
background: palevioletred;
color: white;
`};
` Adding the new @ts-ignore-enable directive at the start does make the error quiet, but this also work // adding @ts-ignore-enable here could work
const Button = styled.button`
${
// @ts-ignore this line is not needed, just remind that the prop could be ignored in this way too
props =>
// @ts-ignore
props.primary && // no error message now!
css`
background: palevioletred;
color: white;
`};
` Do I get your meaning correctly? If not, could you give simple steps or a minimal repository to help me get it? Thanks! |
Aha! I stand corrected. The original code I was fighting with looked more like this (if memory serves): import React from "react";
import styled from "styled-components";
const Button: React.FC<{
// Since this is missing className and style props, it is invalid.
nope: string;
}> = () => <button />;
const Form = styled.form`
${Button} {
color: rebeccapurple;
}
`; The error happens at const Form = styled.form`
${// @ts-ignore
Button} {
color: rebeccapurple;
}
`; As I recall, I was unable to put |
Seems the issue is resolved. |
I remember seeing a PR for that fix in the last couple of months. @ShuiRuTian do you want to keep working on this? If not, we can close it in its current state and restart work again later if something changes. |
I would be glad to continue working on this again when TS team decides to have this feature someday. |
Fixes #19573