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

refactor: all flattenToken need hashed #204

Merged
merged 1 commit into from
Nov 14, 2024
Merged

refactor: all flattenToken need hashed #204

merged 1 commit into from
Nov 14, 2024

Conversation

zombieJ
Copy link
Member

@zombieJ zombieJ commented Nov 14, 2024

fix #197
fix #188
close #198

useGlobalCache 需要走一遍 useMemo cache,deps 由于会被 React Fiber cache 导致内存占用。这里拿时间换一点空间出来,从而节省巨型页面中的内存占用情况。

Summary by CodeRabbit

  • 新功能

    • 改进了令牌处理流程,确保所有扁平化的令牌都经过哈希处理,从而简化了逻辑。
  • 错误修复

    • 测试确保样式在组件的生命周期内正确应用和移除,增强了样式管理系统的健壮性。
  • 文档

    • 更新了测试用例,以反映 flattenToken 函数的行为变化,确保缓存机制正常工作。

Copy link

coderabbitai bot commented Nov 14, 2024

📝 Walkthrough

Walkthrough

本次更改主要涉及 src/util/index.ts 文件中 flattenTokentoken2key 函数的修改。flattenToken 函数的签名更新,移除了 hashed 参数,使得输出字符串始终进行哈希处理,简化了内部逻辑。同时,token2key 函数的调用也相应更新,以确保使用新的 flattenToken 实现。此外,测试文件 tests/index.spec.tsxtests/util.spec.tsx 中的测试用例进行了相应调整,以验证新的令牌处理逻辑和样式管理系统的健壮性。

Changes

文件 更改摘要
src/util/index.ts 更新 flattenToken 函数签名,移除 hashed 参数;更新 token2key 函数以适应新实现。
tests/index.spec.tsx 更新多个测试用例以验证样式注册、令牌衍生、组件行为、嵌套样式、样式卸载、哈希处理等功能的正确性。
tests/util.spec.tsx 更新 flattenToken 函数的预期输出,确认缓存机制正常,保留 memoResult 测试用例不变。

Assessment against linked issues

Objective Addressed Explanation
解决 useStyles 内存泄漏问题 (#197) 本次更改未涉及 useStyles 的内存管理逻辑。
解决 useGlobalCache 内存泄漏问题 (#188) 本次更改未处理 useGlobalCache 的内存泄漏问题。

Poem

🐰 在代码的世界里,令牌轻轻飞,
哈希总是伴随,简化每一堆。
测试如花绽放,样式不再乱,
兔子欢呼雀跃,期待更美的天! 🌼

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 eslint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options.
warning eslint > @humanwhocodes/[email protected]: Use @eslint/config-array instead
warning eslint > @humanwhocodes/config-array > @humanwhocodes/[email protected]: Use @eslint/object-schema instead
warning eslint > file-entry-cache > flat-cache > [email protected]: Rimraf versions prior to v4 are no longer supported
warning eslint > file-entry-cache > flat-cache > rimraf > [email protected]: Glob versions prior to v9 are no longer supported
warning eslint > file-entry-cache > flat-cache > rimraf > glob > [email protected]: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
warning @umijs/fabric > [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options.
warning @umijs/fabric > @babel/[email protected]: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-class-properties instead.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

github-actions bot commented Nov 14, 2024

🎊 PR Preview 2bfc425 has been successfully built and deployed to https://ant-design-cssinjs-preview-pr-204.surge.sh

🕐 Build time: 91.206s

🤖 By surge-preview

if (hashed) {
str = hash(str);
}
str = hash(str);
Copy link
Member

Choose a reason for hiding this comment

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

上面的 #48386 应该是跷跷板问题吧,hash 不缩短,拼接就会很长🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

额,48386 指的是什么?

Copy link
Member

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.

应该不是这个……

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (3)
src/util/index.ts (2)

56-56: 建议添加更详细的注释说明哈希的必要性

当前注释仅提到了样式标签长度问题(#48386),建议补充说明这个改动对解决内存泄漏的帮助。

-    // https://github.com/ant-design/ant-design/issues/48386
-    // Should hash the string to avoid style tag name too long
+    // 1. Hash the string to avoid style tag name too long (#48386)
+    // 2. Consistent hashing helps prevent memory leaks by ensuring token identity

Line range hint 38-60: 缓存策略的实现值得肯定

使用 WeakMap 作为缓存机制是个很好的选择:

  1. 允许未使用的令牌被垃圾回收,防止内存泄漏
  2. 通过分离不同用途的缓存(resultCache 和 flattenTokenCache)保持了关注点分离

这种实现方式很好地平衡了性能和内存管理。

tests/index.spec.tsx (1)

205-205: 建议避免硬编码哈希值

在测试中直接比较硬编码的哈希值可能会导致测试在哈希算法或输入更改时失败。建议使用动态计算的哈希值或使用更健壮的方式进行验证。

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 10c7642 and 2bfc425.

📒 Files selected for processing (3)
  • src/util/index.ts (3 hunks)
  • tests/index.spec.tsx (1 hunks)
  • tests/util.spec.tsx (1 hunks)
🔇 Additional comments (3)
tests/util.spec.tsx (1)

128-128: 测试用例已正确更新以匹配新的哈希实现

测试用例已更新以反映 flattenToken 函数的新行为,现在始终返回哈希值。这与重构目标保持一致。

运行以下脚本以验证是否还有其他使用 flattenToken 的测试用例需要更新:

✅ Verification successful

测试更新验证完成,无需额外修改

搜索结果显示:

  • flattenToken 函数仅在 tests/util.spec.tsx 中被使用一次
  • 这正是我们已经审查的测试用例
  • 没有发现其他需要更新的测试用例
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:查找可能需要更新的其他测试用例

# 搜索所有测试文件中的 flattenToken 调用
rg -t tsx -t ts "flattenToken\(" "tests/"

# 搜索可能包含旧输出格式的断言
rg -t tsx -t ts "expect\(.*flattenToken.*\)" "tests/"

Length of output: 151


Script:

#!/bin/bash
# 使用更通用的文件搜索方式

# 使用 fd 查找所有测试文件
fd "\.(test|spec)\.(ts|tsx|js|jsx)$" tests/

# 使用 ripgrep 搜索 flattenToken 的调用,不指定文件类型
rg "flattenToken\(" tests/

# 搜索包含 flattenToken 的断言语句
rg "expect.*flattenToken" tests/

Length of output: 495

src/util/index.ts (2)

68-68: 令牌键生成逻辑的更新是正确的

移除了不必要的 hashed 参数,同时保持了基于盐值的哈希安全模型。这个改动与 flattenToken 的更改保持一致。


Line range hint 38-48: 强制哈希处理所有令牌的更改是合理的

这个更改通过移除 hashed 参数并始终进行哈希处理,使令牌处理更加一致和可预测。这有助于解决内存泄漏问题。

不过建议验证一下递归实现的安全性。

✅ Verification successful

递归实现是安全的,无需担心循环引用问题

基于代码分析结果,我可以确认 flattenToken 函数的实现是安全的,原因如下:

  1. 使用 WeakMap (flattenTokenCache) 进行缓存,可以有效防止重复处理相同的令牌对象
  2. 测试用例 (tests/util.spec.tsx) 验证了缓存机制的有效性,即使调用 10000 次,getter 也只被调用一次
  3. 递归实现时:
    • 只处理对象的可枚举属性
    • Theme 实例做了特殊处理,直接使用其 id 属性
    • 使用哈希函数处理最终字符串,确保输出长度可控

这些机制共同确保了函数不会陷入无限递归,同时能高效处理嵌套的令牌对象。

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# 描述:检查是否存在可能导致无限递归的循环引用令牌

# 搜索包含循环引用的测试用例
rg -l "const.*=.*{.*:.*}" tests/

Length of output: 145


Script:

#!/bin/bash
# 搜索可能的循环引用模式
rg -A 5 "const\s+\w+\s*=\s*\{" tests/

# 检查测试文件中的 flattenToken 相关测试用例
rg -A 10 "flattenToken" tests/

# 检查源代码中是否有防止循环引用的机制
ast-grep --pattern 'function flattenToken($_) {
  $$$
}'

Length of output: 4974

@zombieJ zombieJ merged commit 846d1b3 into master Nov 14, 2024
7 checks passed
@zombieJ zombieJ deleted the flatten branch November 14, 2024 09:12
@Wxh16144
Copy link
Member

当前的 PR 修复方式和之前有人打补丁的行为一样,只是解决 react hook 的 依赖大小 。

@afc163
Copy link
Member

afc163 commented Nov 14, 2024

修复了 ant-design/ant-design#49885 么?

@jackple
Copy link

jackple commented Nov 25, 2024

修复了 ant-design/ant-design#49885 么?

同问, antd依赖也没update

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.

useStyles产生的内存泄漏问题 useGlobalCache 会产生内存泄漏
5 participants