react-strict-dom icon indicating copy to clipboard operation
react-strict-dom copied to clipboard

Remove legacy ThemeContext export

Open necolas opened this issue 1 year ago • 2 comments
trafficstars

The theme context is a legacy API that was only used on native to set theme variables. We now have a native shim for the StyleX theme APIs, and no longer require the theme context.

This is another PR that shouldn't be merged until we've migrated internal XPR/RN code onto the StyleX theme APIs.

necolas avatar Apr 08 '24 21:04 necolas

compressed-size: runtime library

Size change: -0.10 kB Total size: 18.59 kB

Filename: gzip (minify) kB size kB change % change
./packages/react-strict-dom/dist/dom/index.js 2.89 (8.69) -0.10 (-0.30) -3.4% (-3.4%) 🟢
View unchanged
Filename: gzip (minify) kB size kB change % change
./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 14.74 (46.53) 0.00 (0.00) 0.0% (0.0%)

github-actions[bot] avatar Apr 08 '24 21:04 github-actions[bot]

This is another PR that shouldn't be merged until we've migrated internal XPR/RN code onto the StyleX theme APIs

I don't think is gonna be fully done for months FWIW.

nmn avatar Apr 08 '24 23:04 nmn

This also allows react-strict-dom components to be used as React Server Components in Next.js without babel plugin (babel is buggy in Next.js App Dir so this is quite useful).

javascripter avatar May 07 '24 12:05 javascripter

This PR is only to remove an unused (in OSS) export for native. It shouldn't make any difference to use of Next or RSC on web today

necolas avatar May 07 '24 14:05 necolas

@necolas https://github.com/javascripter/rsd-next-rsc Here's the repro.

I think this PR removes contexts export in dom/index.js too, not just for native, specifically this line: https://github.com/facebook/react-strict-dom/blob/780779a65314e2b65b4ffed0c0284afaff6e8585/packages/react-strict-dom/src/dom/index.js

This contexts export prevents Next.js from loading it as RSC (the below error). Removing the contexts export allows it to be used as RSC (run patch-package in the repro to apply the diff and run yarn dev and now it works in RSC)

Screenshot 2024-05-08 at 0 27 52

javascripter avatar May 07 '24 15:05 javascripter

I updated the patch to remove ThemeContext only from the DOM exports. This should be safe to land without causing regressions inside Meta. The entry module types will differ between web and native, but the theme context can only be imported from the native entry. We can remove the native part later once Meta updates its internal use of StyleX on web to use the StyleX theming APIs.

necolas avatar May 07 '24 20:05 necolas