facebook / react-strict-dom

React Strict DOM (RSD) standardizes the development of styled React components for web and native.
https://facebook.github.io/react-strict-dom
MIT License
3.19k stars 161 forks source link

Replace `React.AbstractComponent` with component type #220

Closed SamChou19815 closed 1 month ago

SamChou19815 commented 1 month ago

In order to adopt react 19's ref-as-prop model, we need to eliminate all the places where they are treated differently. React.AbstractComponent is the worst example of this, and we need to eliminate it.

This PR replaces all of them in react-strict-dom's html.js file with the new component types.

github-actions[bot] commented 1 month ago

workflow: benchmarks/size

Comparison of minified (terser) and compressed (brotli) size results, measured in bytes. Smaller is better. Results Base Patch Ratio
react-strict-dom/dist/dom/index.js
· compressed 2,488 2,488 1.00
· minified 8,586 8,586 1.00
react-strict-dom/dist/dom/runtime.js
· compressed 850 850 1.00
· minified 2,404 2,404 1.00
react-strict-dom/dist/native/index.js
· compressed 15,651 15,651 1.00
· minified 58,387 58,387 1.00
github-actions[bot] commented 1 month ago

workflow: benchmarks/perf (native)

Comparison of performance test results, measured in operations per second. Larger is better. Results Base Patch Ratio
css.create
· small 1,161,641 1,155,207 0.99 -
· small with units 448,764 448,663 1.00 -
· small with variables 669,647 678,713 1.01 +
· several small 328,815 327,898 1.00 -
· large 213,135 212,804 1.00 -
· large with polyfills 148,737 148,425 1.00 -
· complex 102,776 101,399 0.99 -
· unsupported 224,808 226,057 1.01 +
css.createTheme
· simple theme 219,922 219,825 1.00 -
· polyfill theme 207,977 207,148 1.00 -
css.props
· small 247,470 246,608 1.00 -
· small with units 192,866 192,385 1.00 -
· small with variables 106,260 106,054 1.00 -
· small with variables of units 75,498 74,865 0.99 -
· large 103,767 104,727 1.01 +
· large with polyfills 37,599 37,326 0.99 -
· complex 24,169 24,031 0.99 -
· unsupported 149,018 147,318 0.99 -
· simple merge 165,428 162,274 0.98 -
· wide merge 18,395 18,109 0.98 -
· deep merge 18,052 17,819 0.99 -
· themed merge 32,104 31,767 0.99 -
necolas commented 1 month ago

(BTW please leave merges to maintainers)

The TS types before:

export declare const a: React.ComponentType<
  StrictReactDOMAnchorProps & React.RefAttributes<HTMLAnchorElement>
>;

After:

export declare const a: (
  props: Omit<
    StrictReactDOMAnchorProps,
    keyof ({ ref?: React.Ref<HTMLAnchorElement> })
  > & { ref?: React.Ref<HTMLAnchorElement> }
) => React.ReactNode;
nmn commented 1 month ago

This type:

export declare const a: (
  props: Omit<
    StrictReactDOMAnchorProps,
    keyof ({ ref?: React.Ref<HTMLAnchorElement> })
  > & { ref?: React.Ref<HTMLAnchorElement> }
) => React.ReactNode;

... could be simplified to:

export declare const a: (
  props: Omit<
    StrictReactDOMAnchorProps,
    'ref',
  > & { ref?: React.Ref<HTMLAnchorElement> }
) => React.ReactNode;

And since StrictReactDOMAnchorProps doesn't have a ref key in the type defs, this could be further simplified, to just:

export declare const a: (
  props: 
    StrictReactDOMAnchorProps &
    { ref?: React.Ref<HTMLAnchorElement> }
) => React.ReactNode;

This should be correct, as long as the React TS types allow such functions to be components. If that is not the case, the TS types would need to be updated for React 19.


We should be able to update flow-api-translator to automatically simplify:

keyof ({ ref?: React.Ref<HTMLAnchorElement> })

to

'ref'