Skip to content

refactor: improve linear-weight-sum performance #1216

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

PupilTong
Copy link
Collaborator

No description provided.

@PupilTong PupilTong requested a review from Copilot July 4, 2025 06:47
@PupilTong PupilTong self-assigned this Jul 4, 2025
Copy link

changeset-bot bot commented Jul 4, 2025

🦋 Changeset detected

Latest commit: 8227b08

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@lynx-js/web-style-transformer Patch
@lynx-js/web-elements Patch
@lynx-js/web-core Patch
@lynx-js/web-mainthread-apis Patch
@lynx-js/web-core-server Patch
@lynx-js/web-worker-runtime Patch
@lynx-js/web-constants Patch
@lynx-js/web-worker-rpc Patch
@lynx-js/web-elements-template Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@PupilTong PupilTong requested a review from Sherry-hue July 4, 2025 06:47
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the handling of linear-weight-sum by removing the JS fallback for zero values and moving that logic into CSS, using a clamp-based calculation to default the sum to 1 when it's zero.

  • Simplified transformLynxStyles.ts by always emitting the raw value for --lynx-linear-weight-sum
  • Updated linear.css to use nested calc + clamp to avoid division by zero at runtime
  • Bumped versions via a changeset

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
packages/web-platform/web-style-transformer/src/transformLynxStyles.ts Remove JS check and override for 0 values on linear-weight-sum
packages/web-platform/web-elements/src/common-css/linear.css Add nested calc + clamp to default zero sum to 1 in CSS
.changeset/hungry-hoops-sin.md Version patch bump for the refactor

linearWeightSum = '1';
}
childStyle.push(['--lynx-linear-weight-sum', linearWeightSum]);
childStyle.push(['--lynx-linear-weight-sum', value]);
Copy link
Preview

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

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

Passing values like '0 !important' directly into the custom property will break CSS arithmetic (e.g., clamp and multiplication). Consider stripping out !important from the value and setting it via the style priority argument instead.

Suggested change
childStyle.push(['--lynx-linear-weight-sum', value]);
const important = value.includes('!important');
const sanitizedValue = value.replace('!important', '').trim();
childStyle.push(['--lynx-linear-weight-sum', sanitizedValue]);
// Optionally handle `important` if needed in the future

Copilot uses AI. Check for mistakes.

@@ -285,7 +285,13 @@ list-item {
list-item {
flex-shrink: 0;
Copy link
Preview

Copilot AI Jul 4, 2025

Choose a reason for hiding this comment

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

[nitpick] The nested calc + clamp expression to default zero sums to 1 is non-obvious. Add a brief comment explaining the multiplier and clamp hack to help future maintainers understand the intent.

Suggested change
flex-shrink: 0;
flex-shrink: 0;
/* The following `calc` and `clamp` logic ensures that if
`--lynx-linear-weight-sum` is zero, it defaults to 1. This prevents
division by zero and ensures consistent behavior. */

Copilot uses AI. Check for mistakes.

Copy link

codspeed-hq bot commented Jul 4, 2025

CodSpeed Performance Report

Merging #1216 will not alter performance

Comparing PupilTong:p/hw/weight-use-rename (8227b08) with main (4097b3f)

Summary

✅ 10 untouched benchmarks

Copy link

relativeci bot commented Jul 4, 2025

React Example

#2510 Bundle Size — 234.7KiB (0%).

8227b08(current) vs 4097b3f main#2501(baseline)

Bundle metrics  no changes
                 Current
#2510
     Baseline
#2501
No change  Initial JS 0B 0B
No change  Initial CSS 0B 0B
No change  Cache Invalidation 0% 0%
No change  Chunks 0 0
No change  Assets 4 4
No change  Modules 153 153
No change  Duplicate Modules 61 61
No change  Duplicate Code 45.85% 45.85%
No change  Packages 2 2
No change  Duplicate Packages 0 0
Bundle size by type  no changes
                 Current
#2510
     Baseline
#2501
No change  IMG 145.76KiB 145.76KiB
No change  Other 88.94KiB 88.94KiB

Bundle analysis reportBranch PupilTong:p/hw/weight-use-renameProject dashboard


Generated by RelativeCIDocumentationReport issue

Copy link

relativeci bot commented Jul 4, 2025

Web Explorer

#2501 Bundle Size — 259.22KiB (~+0.01%).

8227b08(current) vs 4097b3f main#2492(baseline)

Bundle metrics  Change 3 changes Regression 1 regression
                 Current
#2501
     Baseline
#2492
No change  Initial JS 140.64KiB 140.64KiB
Regression  Initial CSS 31.88KiB(+0.19%) 31.82KiB
Change  Cache Invalidation 23.05% 0%
No change  Chunks 4 4
No change  Assets 4 4
Change  Modules 207(-0.48%) 208
No change  Duplicate Modules 16 16
No change  Duplicate Code 3.61% 3.61%
No change  Packages 4 4
No change  Duplicate Packages 0 0
Bundle size by type  Change 2 changes Regression 1 regression Improvement 1 improvement
                 Current
#2501
     Baseline
#2492
Improvement  JS 227.34KiB (-0.02%) 227.39KiB
Regression  CSS 31.88KiB (+0.19%) 31.82KiB

Bundle analysis reportBranch PupilTong:p/hw/weight-use-renameProject dashboard


Generated by RelativeCIDocumentationReport issue

@PupilTong PupilTong force-pushed the p/hw/weight-use-rename branch from ccc1bd9 to ac25977 Compare July 4, 2025 10:16
@PupilTong PupilTong force-pushed the p/hw/weight-use-rename branch from ac25977 to 8227b08 Compare July 4, 2025 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

1 participant