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: useMemoizedFn: declare this explicitly #1464

Merged
merged 1 commit into from
Feb 15, 2022
Merged

chore: useMemoizedFn: declare this explicitly #1464

merged 1 commit into from
Feb 15, 2022

Conversation

fanck0605
Copy link
Contributor

[English Template / 英文模板]

🤔 这个变动的性质是?

  • 代码风格优化

💡 需求背景和解决方案

显式地声明 this,无需强制关闭 eslint 了。

📝 更新日志

语言 更新描述
🇺🇸 英文
🇨🇳 中文

☑️ 请求合并前的自查清单

⚠️ 请自检并全部勾选全部选项⚠️

  • 文档已补充或无须补充
  • 代码演示已提供或无须提供
  • TypeScript 定义已补充或无须补充
  • Changelog 已提供或无须提供

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@fanck0605 fanck0605 changed the title useMemoizedFn: declare this explicitly chore: useMemoizedFn: declare this explicitly Feb 14, 2022
@@ -17,8 +17,7 @@ function useMemoizedFn<T extends noop>(fn: T) {

const memoizedFn = useRef<T>();
if (!memoizedFn.current) {
memoizedFn.current = function (...args) {
// eslint-disable-next-line @typescript-eslint/no-invalid-this
memoizedFn.current = function (this, ...args) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

这是用户在调用函数的时候,需要给第一个参数传递 this 吗?

Copy link
Contributor Author

@fanck0605 fanck0605 Feb 15, 2022

Choose a reason for hiding this comment

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

不需要,这里的 this 仅声明,调用函数只需要传递 args,编译成 js 后就没有 this 了

https://www.typescriptlang.org/docs/handbook/2/functions.html#declaring-this-in-a-function
可以在线预览一下编译后的结果

Copy link
Collaborator

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

不需要,这里的 this 仅声明,调用函数只需要传递 args,编译成 js 后就没有 this 了

https://www.typescriptlang.org/docs/handbook/2/functions.html#declaring-this-in-a-function 可以在线预览一下编译后的结果

原来如此~

Copy link
Collaborator

@brickspert brickspert left a comment

Choose a reason for hiding this comment

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

LGTM

@brickspert brickspert merged commit cbc8f8f into alibaba:master Feb 15, 2022
@brickspert
Copy link
Collaborator

这个改动之后,其它使用到 useMemoizedFn 的地方,编译后的 ts 类型都不对了。
image

虽然可以通过在 useMemoizedFn 中强制指定返回的数据类型,但整体的可读性确实不好了。

return memoizedFn.current as T;

我先把这个改定 revert 了。

@brickspert
Copy link
Collaborator

这个改动之后,其它使用到 useMemoizedFn 的地方,编译后的 ts 类型都不对了。 image

虽然可以通过在 useMemoizedFn 中强制指定返回的数据类型,但整体的可读性确实不好了。

return memoizedFn.current as T;

我先把这个改定 revert 了。

错了,不是这个 PR 引起的。 是另外一个 #1470

@fanck0605
Copy link
Contributor Author

抱歉,因为我的考虑不周导致了新的问题😵。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants