-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Update typescript-eslint v8 #16557
Update typescript-eslint v8 #16557
Conversation
liuxingbaoyu
commented
Jun 5, 2024
Q | A |
---|---|
Fixed Issues? | Closes #15547 |
Patch: Bug Fix? | |
Major: Breaking Change? | |
Minor: New Feature? | |
Tests Added + Pass? | Yes |
Documentation PR Link | |
Any Dependency Changes? | |
License | MIT |
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/57171 |
c6ae203
to
9c7bd9c
Compare
I can't reproduce this error locally, which is weird. |
I also cannot reproduce the failure locally 🤔 @JoshuaKGoldberg I couldn't find the typescript-eslint v8 changelog -- Should |
@@ -10,7 +10,7 @@ import type { FileResult, InputOptions } from "@babel/core"; | |||
export function chmod(src: string, dest: string): void { | |||
try { | |||
fs.chmodSync(dest, fs.statSync(src).mode); | |||
} catch (err) { | |||
} catch (_) { |
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.
[Question] Is there a reason not to:
} catch (_) { | |
} catch { |
...since you're transpiling from TypeScript anyway?
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.
We only enabled the typescript plugin to avoid accidental auto-transformations increasing the output size. :)
Ah, we haven't set one up yet. For now the typescript-eslint 8.0.0 milestone is roughly that.
Though, note that
That's a good question. I haven't found any discussion on what to do with If you think the rule shouldn't report on |
fcf0021
to
d54e0e3
Compare
Maybe we can try it again after merging #16536. |
@liuxingbaoyu TS 5.5 PR merged, can you try rebasing? :) |
d54e0e3
to
b1a1c89
Compare
Of course! |
This reverts commit b1a1c89.
@@ -96,7 +96,7 @@ function AsyncFromSyncIterator<T, TReturn = any, TNext = undefined>(s: any) { | |||
}, | |||
throw: function (maybeError?: any) { | |||
var thr = this.s.return; | |||
if (thr === undefined) return Promise.reject(maybeError); | |||
if (thr === undefined) return Promise.reject(maybeError as Error); |
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.
Given that maybeError
here could be also something that is not an error, I would prefer to use an eslint-disable-next-line
comment rather than a "wrong" cast.