-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Repo: use default TypeScript compilerOptions.lib
when testing rules
#9648
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
Comments
Hmm, I'm [edit: not] a huge fan of adding globals in implicitly. I think a cleaner direction would be to manually change |
Guessing you missed a Anyways, I'm fine either way, but a couple points to consider —
On the other hand, if we do not add
|
Heh, "why not both?" - the default settings of TS are to implicitly add in globals! (not disputing thinking of it as one or the other, just amused)
I'd say that's a good general rule, but so is being overly precautious. Sometimes we in typescript-eslint accidentally use DOM/etc. globals we shouldn't, as you've noted.
Because we're sloppy 😄. We context switch a lot and take in contributions from people who have even less context than us. Those are bound to get messed up a whole bunch. To me, that discrepancy is a symptom of the issue! |
Following up: let's switch the playground to default to not having |
Suggestion
Right now, in our rule tests for
eslint-plugin
, we use a non-default value for thecompilerOptions.lib
option, that, in particular, removesDOM
.This is different from the
compilerOptions.lib
used on the playground, which does includeDOM
. Because of this mismatch, the "Invalid" snapshots fromdocs-eslint-output-snapshots
for two rules are missing ESLint errors that do show up on the playground:no-confusing-void-expression
(because of missing types foralert()
andpostMessage()
no-misused-promises
(because of missing types foraddEventListener()
)The text was updated successfully, but these errors were encountered: