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

Attempt to use Flow's new StringPrefix feature #171

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

Conversation

necolas
Copy link
Contributor

@necolas necolas commented Jul 20, 2024

Fix #71

Copy link

compressed-size: runtime library

Size change: 0.00 kB
Total size: 20.35 kB

View unchanged
Filename: gzip (minify) kB size kB change % change
./packages/react-strict-dom/dist/dom/index.js 2.94 (8.79) 0.00 (0.00) 0.0% (0.0%)
./packages/react-strict-dom/dist/dom/runtime.js 0.95 (2.33) 0.00 (0.00) 0.0% (0.0%)
./packages/react-strict-dom/dist/native/index.js 16.45 (53.21) 0.00 (0.00) 0.0% (0.0%)

Copy link

workflow: benchmarks (native)

Comparison of benchmark test results, measured in operations per second (higher is faster.)

Suite Base Patch Ratio
css.create
· small 1,147,308 1,152,530 1.00 +
· small with units 449,318 448,615 1.00 -
· small with variables 685,851 673,815 0.98 -
· several small 328,188 330,084 1.01 +
· large 214,801 216,107 1.01 +
· large with polyfills 154,010 152,267 0.99 -
· complex 105,327 104,393 0.99 -
· unsupported 231,619 233,296 1.01 +
css.createTheme
· simple theme 226,291 225,584 1.00 -
· polyfill theme 213,384 213,392 1.00 +
css.props
· small 235,566 237,887 1.01 +
· small with units 186,777 187,395 1.00 +
· small with variables 105,128 105,478 1.00 +
· small with variables of units 75,260 75,083 1.00 -
· large 102,550 102,049 1.00 -
· large with polyfills 25,978 26,162 1.01 +
· complex 18,084 18,224 1.01 +
· unsupported 75,234 75,443 1.00 +
· simple merge 160,836 160,580 1.00 -
· wide merge 13,748 13,875 1.01 +
· deep merge 13,561 13,650 1.01 +
internals
· extractStyleThemes 425,401 424,882 1.00 -
· flattenStyle 654,521 654,518 1.00 -

@@ -280,9 +280,9 @@ type EventProps = $ReadOnly<{
}>;

export type StrictReactDOMProps = $ReadOnly<{
[StringPrefix<'data-'>]: ?string,
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you check if the generated TS types work as expected?

If so, this LGTM.

@nmn
Copy link
Contributor

nmn commented Jul 21, 2024

There seems to be a Flow bug with the StringPrefix type still when being used in an object type as in the PR.

@necolas
Copy link
Contributor Author

necolas commented Jul 21, 2024

This is not ready to merge, I put it up for the flow team. We've been talking about the issues with the API and it's translation to TS. They're looking into it

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

Successfully merging this pull request may close these issues.

Flow type support for data-* props
3 participants