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

more granular unparsable prop detection for StyleInfo #6631

Merged
merged 4 commits into from
Nov 12, 2024

Conversation

bkrmendy
Copy link
Contributor

@bkrmendy bkrmendy commented Nov 11, 2024

Problem

The StyleInfo type doesn't express the state when a property is set, but it cannot parsed in the desired format. For example, in style={{ top: '300px', left: theme.left + 50 }}, top can be parsed as a css number, but left cannot be.

We need to be able to express this, since some code (AdjustCssLengthProperties and SetCSSLength for example) need to handle it. Once AdjustCssLengthProperties and SetCSSLength use this infra, we can make more strategies Tailwind-compatible.

Fix

Extend the StyleInfo type, and fix any affected code.

Manual Tests:
I hereby swear that:

  • I opened a hydrogen project and it loaded
  • I could navigate to various routes in Play mode

Copy link
Contributor

github-actions bot commented Nov 11, 2024

Try me

Copy link

relativeci bot commented Nov 11, 2024

#15096 Bundle Size — 58.06MiB (~+0.01%).

f2db984(current) vs d1d46c1 master#15094(baseline)

Warning

Bundle contains 70 duplicate packages – View duplicate packages

Bundle metrics  Change 2 changes Regression 1 regression
                 Current
#15096
     Baseline
#15094
Regression  Initial JS 41.04MiB(~+0.01%) 41.04MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 18.11% 18%
No change  Chunks 20 20
No change  Assets 22 22
No change  Modules 4167 4167
No change  Duplicate Modules 213 213
No change  Duplicate Code 27.3% 27.3%
No change  Packages 477 477
No change  Duplicate Packages 70 70
Bundle size by type  Change 2 changes Regression 1 regression Improvement 1 improvement
                 Current
#15096
     Baseline
#15094
Regression  JS 58.06MiB (~+0.01%) 58.06MiB
Improvement  HTML 7.37KiB (-0.25%) 7.39KiB

Bundle analysis reportBranch feature/style-info-not-editable-...Project dashboard


Generated by RelativeCIDocumentationReport issue

@bkrmendy bkrmendy marked this pull request as ready for review November 11, 2024 15:54
@bkrmendy bkrmendy changed the title more granular non-editable prop detection for StyleInfo more granular unparsable prop detection for StyleInfo Nov 11, 2024
@bkrmendy bkrmendy merged commit a668e00 into master Nov 12, 2024
15 checks passed
@bkrmendy bkrmendy deleted the feature/style-info-not-editable-property branch November 12, 2024 13:35
liady pushed a commit that referenced this pull request Dec 13, 2024
## Problem
The `StyleInfo` type doesn't express the state when a property is set,
but it cannot parsed in the desired format. For example, in `style={{
top: '300px', left: theme.left + 50 }}`, `top` can be parsed as a css
number, but left cannot be.

We need to be able to express this, since some code
(`AdjustCssLengthProperties` and `SetCSSLength` for example) need to
handle it. Once `AdjustCssLengthProperties` and `SetCSSLength` use this
infra, we can make more strategies Tailwind-compatible.

## Fix
Extend the `StyleInfo` type, and fix any affected code. 

**Manual Tests:**
I hereby swear that:

- [x] I opened a hydrogen project and it loaded
- [x] I could navigate to various routes in Play mode
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.

4 participants