Skip to content
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

chore: 🆙 upgrade typescript-eslint #26600

Merged
merged 13 commits into from
Sep 16, 2020
Merged

chore: 🆙 upgrade typescript-eslint #26600

merged 13 commits into from
Sep 16, 2020

Conversation

afc163
Copy link
Member

@afc163 afc163 commented Sep 5, 2020

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English
🇨🇳 Chinese

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

@ant-design-bot
Copy link
Contributor

ant-design-bot commented Sep 5, 2020

@ant-design-bot
Copy link
Contributor

ant-design-bot commented Sep 5, 2020

@afc163
Copy link
Member Author

afc163 commented Sep 5, 2020

/rebase

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 5, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 9d676eb:

Sandbox Source
antd reproduction template Configuration

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2020

Size Change: -2 B (0%)

Total Size: 876 kB

Filename Size Change
./dist/antd-with-locales.min.js 355 kB -2 B (0%)
ℹ️ View Unchanged
Filename Size Change
./dist/antd.compact.min.css 66.4 kB 0 B
./dist/antd.dark.min.css 67.6 kB 0 B
./dist/antd.min.css 66.4 kB 0 B
./dist/antd.min.js 321 kB 0 B

compressed-size-action

@afc163
Copy link
Member Author

afc163 commented Sep 5, 2020

Need help to fix new eslint errors.

@abenhamdine
Copy link
Contributor

I would recommend not upgrading to typescript-eslint 4 until these annoying false positives are fixed, see https://github.com/typescript-eslint/typescript-eslint/milestone/7

@kerm1it
Copy link
Member

kerm1it commented Sep 5, 2020

image

Obviously this isn't problem of code.

I think we can upgrade them when they are stable.

@afc163 afc163 added the help wanted The suggestion or request has been accepted, we need you to help us by sending a pull request. label Sep 7, 2020
@afc163
Copy link
Member Author

afc163 commented Sep 7, 2020

/rebase

@abenhamdine
Copy link
Contributor

abenhamdine commented Sep 8, 2020

For information, https://github.com/typescript-eslint/typescript-eslint/releases/tag/v4.1.0 has been released and fixes 90% of v4 false positives, but I still see a few remaining false positives in our own codebase at @Arhia.

@yoyo837
Copy link
Contributor

yoyo837 commented Sep 8, 2020

Need to continue to wait for version 4.1.1 or higher...

@afc163 afc163 marked this pull request as draft September 10, 2020 07:25
@abenhamdine
Copy link
Contributor

Actually, community feedbacks show there more and more false positives detected, 4.X is definitely not currently usable.

@hengkx
Copy link
Member

hengkx commented Sep 14, 2020

/rebase

@pr-triage pr-triage bot added the PR: draft label Sep 15, 2020
@afc163
Copy link
Member Author

afc163 commented Sep 15, 2020

/rebase

@hengkx
Copy link
Member

hengkx commented Sep 15, 2020

@afc163
Copy link
Member Author

afc163 commented Sep 15, 2020

/rebase

@hengkx
Copy link
Member

hengkx commented Sep 16, 2020

/rebase

@hengkx
Copy link
Member

hengkx commented Sep 16, 2020

/rebase

@@ -33,7 +33,7 @@ export interface ModalStaticFunctions {
export default function confirm(config: ModalFuncProps) {
const div = document.createElement('div');
document.body.appendChild(div);
// eslint-disable-next-line no-use-before-define
// eslint-disable-next-line @typescript-eslint/no-use-before-define
let currentConfig = { ...config, close, visible: true } as any;
Copy link
Member

@hengkx hengkx Sep 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我也想知道 close 是从什么地方 来的 没看懂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

window.close?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

window.close?

不是,就是一个close的props

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

可以提到前面来:

function close(...args: any[]) {
currentConfig = {
...currentConfig,
visible: false,
afterClose: destroy.bind(this, ...args),
};
render(currentConfig);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

window.close?

不是,就是一个close的props

根本 没有 定义。。。

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

看我的回复。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

提不了吧 互相 引用了

@hengkx
Copy link
Member

hengkx commented Sep 16, 2020

lint 终于 过了🥳

@codecov
Copy link

codecov bot commented Sep 16, 2020

Codecov Report

Merging #26600 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #26600   +/-   ##
=======================================
  Coverage   99.82%   99.82%           
=======================================
  Files         384      384           
  Lines        7371     7371           
  Branches     2009     2050   +41     
=======================================
  Hits         7358     7358           
  Misses         13       13           
Impacted Files Coverage Δ
components/_util/type.ts 100.00% <ø> (ø)
components/anchor/Anchor.tsx 100.00% <ø> (ø)
components/avatar/index.tsx 100.00% <ø> (ø)
components/checkbox/Checkbox.tsx 100.00% <ø> (ø)
components/modal/confirm.tsx 100.00% <ø> (ø)
components/radio/index.tsx 100.00% <ø> (ø)
components/transfer/index.tsx 100.00% <ø> (ø)
components/auto-complete/index.tsx 100.00% <100.00%> (ø)
components/date-picker/generatePicker/index.tsx 100.00% <100.00%> (ø)
components/form/hooks/useForm.ts 100.00% <100.00%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0de1358...9d676eb. Read the comment docs.

@hengkx hengkx marked this pull request as ready for review September 16, 2020 02:00
@@ -1,4 +1,4 @@
/* eslint no-use-before-define: "off" */
/* eslint @typescript-eslint/no-use-before-define: "off" */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个还需要么

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

需要

@@ -121,6 +121,14 @@ module.exports = {
'unicorn/prefer-trim-start-end': 2,
'unicorn/expiring-todo-comments': 2,
'unicorn/no-abusive-eslint-disable': 2,

// https://github.com/typescript-eslint/typescript-eslint/issues/2540#issuecomment-692866111
'no-use-before-define': 0,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

为什么他们不通过覆盖config的方式来关闭,要用户自己配置呢?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我也不知道 也没 找到 其它 好的 方案了 ,只能 这样先 干掉,看了下 ts 的确 是正确的 引用

@afc163 afc163 merged commit 481fd20 into master Sep 16, 2020
@afc163 afc163 deleted the typescript-eslint branch September 16, 2020 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted The suggestion or request has been accepted, we need you to help us by sending a pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants