Skip to content

feat: avoid layouts thrashing#3764

Merged
bytemain merged 25 commits intomainfrom
feat/avoid-re-render
Sep 2, 2024
Merged

feat: avoid layouts thrashing#3764
bytemain merged 25 commits intomainfrom
feat/avoid-re-render

Conversation

@bytemain
Copy link
Copy Markdown
Member

@bytemain bytemain commented Jun 12, 2024

Types

  • 🎉 New Features

Background or solution

  1. 使用 fastdom 来批量化写入和读取操作
    当我们在获取浏览器的布局时,如果当前有未应用的布局,浏览器会强制应用该布局。

所以我们其实不太需要每一次都获取最新的布局,仅获取上一帧的布局信息即可。

使用一个工具包 fastdom 来做到这一点。

  1. 原来的 monaco 的渲染时机是在 react 渲染完的下一帧,现在我们把 resize 的渲染也放在下一帧即可。

Changelog

avoid layouts thrashing

Summary by CodeRabbit

  • 新功能

    • 引入了 fastdom 以优化 DOM 操作的性能。
    • 增强了事件处理机制,通过指令来处理 ResizeEvent 和 GridResizeEvent。
    • 新增了 ResizeObserverWrapper 类,用于监测 HTML 元素的尺寸变化。
  • 性能改进

    • 在多个组件中实现了 fastdom.measurefastdom.measureAtNextFrame,优化了 DOM 测量和操作。
    • 重构了编辑器和服务的布局管理逻辑,以提高性能和响应速度。
    • BrowserCodeEditor 类的资源管理和布局处理得到了改进。
  • 配置变更

    • 编辑器的默认配置中 automaticLayout 属性被设置为 false,用户需手动调整布局设置。
  • 文档更新

    • 更新了多个组件和服务的事件处理逻辑,提升了代码的可维护性和响应速度。

@railway-app railway-app Bot temporarily deployed to core-pr-3764 June 12, 2024 02:35 Destroyed
@railway-app
Copy link
Copy Markdown

railway-app Bot commented Jun 12, 2024

🚅 Previously deployed to Railway in the core project. Environment has been deleted.

@opensumi opensumi Bot added the 🎨 feature feature required label Jun 12, 2024
@railway-app railway-app Bot temporarily deployed to core-pr-3764 July 22, 2024 03:09 Destroyed
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 22, 2024

Warning

Rate limit exceeded

@bytemain has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 23 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Commits

Files that changed from the base of the PR and between e43d2f8 and 2b3d605.

Walkthrough

本次更改引入了一系列性能优化和功能增强,特别是在DOM操作、事件处理和布局管理方面。通过集成fastdom库,多个组件的DOM测量和变更处理得到了改进,减少了布局抖动,从而提高了用户交互的流畅性。此外,新的指令机制增强了事件总线的功能,使得对大小调整事件的处理更加高效和灵活。整体来看,这些更新提升了应用程序的响应能力和可维护性。

Changes

文件 变更摘要
packages/core-browser/.../fastdom.test.ts 新增测试文件,模拟requestAnimationFrame行为,验证fastdom模块的功能。
packages/core-browser/package.json 移除依赖项font-awesomereact-autosize-textarea,简化包依赖。
packages/core-browser/src/bootstrap/... App函数中新增对ResizeEvent指令的调用,增强响应能力。
packages/core-browser/src/components/... 修改SplitPanelresize.tsx,更新事件监听和DOM操作,提升性能和事件处理能力。
packages/core-browser/src/dom/fastdom.ts 新增文件实现高效管理DOM操作,定义DomOperation类和相关方法。
packages/core-browser/src/dom/index.ts 新增fastdom的导入声明,以便其他模块使用。
packages/core-browser/src/layout/... 更新useViewState钩子以更好地处理视图状态变化,集成throttlefastdom
packages/core-common/src/event-bus/... IEventBus接口新增onDirectivefireDirective方法,增强指令处理能力。
packages/editor/src/browser/... 多个文件中更新了编辑器的布局管理和配置,移除了automaticLayout属性,优化了性能。
packages/utils/... 新增Heap数据结构,提供优先级队列的实现,增强了数据管理能力。
tools/dev-tool/src/index.html index.html中新增<link rel="preconnect">元素,以优化外部资源加载性能。
packages/core-browser/src/dom/resize-observer.ts 新增ResizeObserverWrapper类,实现对HTML元素尺寸变化的观察功能。

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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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
Copy Markdown
Contributor

@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: 1

Outside diff range, codebase verification and nitpick comments (2)
packages/core-browser/src/dom/fastdom.ts (1)

16-17: 改进错误处理。

catch 块中,可以考虑记录错误日志或采取其他措施,而不仅仅是调用 onUnexpectedError

- onUnexpectedError(error);
+ console.error('DomOperation error:', error);
+ onUnexpectedError(error);
packages/editor/src/browser/workbench-editor.service.ts (1)

797-809: 使用 fastdom 进行布局更新是个好做法

使用 fastdom 来批量处理 DOM 变更可以避免布局抖动。确保事件监听器在不再需要时被正确地销毁,以避免内存泄漏。

+    // 确保事件监听器在不再需要时被销毁
+    this.toDispose.push(toDispose);

Comment thread packages/core-browser/src/dom/fastdom.ts Outdated
Copy link
Copy Markdown
Contributor

@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: 1

@railway-app railway-app Bot temporarily deployed to core-pr-3764 July 22, 2024 11:22 Destroyed
@railway-app railway-app Bot temporarily deployed to core-pr-3764 July 22, 2024 11:41 Destroyed
Copy link
Copy Markdown
Contributor

@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, codebase verification and nitpick comments (1)
packages/editor/src/browser/workbench-editor.service.ts (1)

797-808: 请确保正确处理 fastdom.mutate 的返回值。

fastdom.mutate 中,返回值是一个取消函数,应该在适当的地方调用以避免潜在的内存泄漏。以下文件中存在类似的问题:

  • packages/editor/src/browser/workbench-editor.service.ts
  • packages/core-browser/src/components/resize/resize.tsx

请检查这些文件,并确保所有 fastdom.mutate 调用的返回值都得到了正确处理。

Analysis chain

请确保正确处理 fastdom.mutate 的返回值。

fastdom.mutate 中,返回值是一个取消函数,应该在适当的地方调用以避免潜在的内存泄漏。

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all usages of `fastdom.mutate` to ensure proper handling of return values.

# Test: Search for the function usage. Expect: Proper handling of return values.
rg --type ts -A 5 $'fastdom.mutate'

Length of output: 5486

@railway-app railway-app Bot temporarily deployed to core-pr-3764 July 22, 2024 12:03 Destroyed
@bytemain
Copy link
Copy Markdown
Member Author

@coderabbitai configuration

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 23, 2024

CodeRabbit configuration
language: zh-CN
tone_instructions: ''
early_access: true
enable_free_tier: true
reviews:
  profile: chill
  request_changes_workflow: false
  high_level_summary: true
  high_level_summary_placeholder: '@coderabbitai summary'
  auto_title_placeholder: '@coderabbitai'
  review_status: false
  poem: false
  collapse_walkthrough: false
  sequence_diagrams: true
  path_filters: []
  path_instructions: []
  abort_on_close: true
  auto_review:
    enabled: true
    auto_incremental_review: true
    ignore_title_keywords: []
    labels: []
    drafts: false
    base_branches: []
  tools:
    shellcheck:
      enabled: true
    ruff:
      enabled: true
    markdownlint:
      enabled: true
    github-checks:
      enabled: true
      timeout_ms: 90000
    languagetool:
      enabled: true
      enabled_only: false
      level: default
      enabled_rules: []
      disabled_rules:
        - EN_UNPAIRED_BRACKETS
        - EN_UNPAIRED_QUOTES
      enabled_categories: []
      disabled_categories:
        - TYPOS
        - TYPOGRAPHY
        - CASING
    biome:
      enabled: true
    hadolint:
      enabled: true
    swiftlint:
      enabled: true
    phpstan:
      enabled: true
      level: default
    golangci-lint:
      enabled: true
    yamllint:
      enabled: true
    gitleaks:
      enabled: true
    checkov:
      enabled: true
    detekt:
      enabled: true
    eslint:
      enabled: true
    ast-grep:
      packages: []
      rule_dirs: []
      util_dirs: []
      essential_rules: true
chat:
  auto_reply: true
knowledge_base:
  opt_out: false
  learnings:
    scope: auto
  issues:
    scope: auto
  jira:
    project_keys: []
  linear:
    team_keys: []

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 30, 2024

Codecov Report

Attention: Patch coverage is 72.01646% with 68 lines in your changes missing coverage. Please review.

Project coverage is 54.73%. Comparing base (4b389d9) to head (2b3d605).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
packages/core-browser/src/dom/resize-observer.ts 0.00% 23 Missing and 2 partials ⚠️
packages/core-browser/src/layout/layout-hooks.ts 31.57% 10 Missing and 3 partials ⚠️
...ges/editor/src/browser/workbench-editor.service.ts 38.09% 10 Missing and 3 partials ⚠️
...-layout/src/browser/accordion/accordion.service.ts 0.00% 4 Missing and 1 partial ⚠️
...s/terminal-next/src/browser/terminal.controller.ts 0.00% 2 Missing and 1 partial ⚠️
packages/utils/src/heap.ts 93.47% 3 Missing ⚠️
...ackages/editor/src/browser/preference/converter.ts 94.11% 2 Missing ⚠️
packages/editor/src/browser/types.ts 33.33% 2 Missing ⚠️
packages/core-browser/src/dom/fastdom.ts 98.30% 1 Missing ⚠️
packages/editor/src/browser/grid/grid.service.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3764      +/-   ##
==========================================
+ Coverage   54.71%   54.73%   +0.01%     
==========================================
  Files        1570     1573       +3     
  Lines       95968    96094     +126     
  Branches    19694    19694              
==========================================
+ Hits        52511    52599      +88     
- Misses      36088    36123      +35     
- Partials     7369     7372       +3     
Flag Coverage Δ
jsdom 50.21% <68.31%> (+0.01%) ⬆️
node 15.47% <25.92%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment thread packages/core-browser/src/dom/resize-observer.ts Outdated
Copy link
Copy Markdown
Contributor

@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: 1

Comment thread packages/core-browser/src/dom/resize-observer.ts
erha19
erha19 previously approved these changes Aug 30, 2024
@bytemain bytemain merged commit 8220550 into main Sep 2, 2024
@bytemain bytemain deleted the feat/avoid-re-render branch September 2, 2024 09:47
bytemain added a commit that referenced this pull request Sep 4, 2024
@coderabbitai coderabbitai Bot mentioned this pull request Jan 8, 2025
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🎨 feature feature required

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants