[Proposal] Allow StyleSheets to be unloaded #15899
Comments
|
🤦🏻 Well, all the time I spent digging through ReactNative's source code for a cache was wasted:
Right now you only need to unset the cache in JS to unregister something and clear up the memory. Making that propagate to native would just be implemented at the same time that someone implements that (not implemented yet) behaviour. Now the only problem is that the |
|
You can find the source code for this class in the React repo: https://github.com/facebook/react/blob/master/src/renderers/native/ReactNativePropRegistry.js |
|
Oh, a ReactNative class is part of React. That's... unexpected. Ok, I'll come up with a PR in react when I have some time for it. It's part of the native renderer so it probably makes sense to leave this issue open rather than creating something in the react repo. |
- withMaterialStyles accepts a mapThemeToStyles function that is passed the materialTheme and should return style objects in the same format you pass to StyleSheet.create - The return of withMaterialStyles is a function that returns a HOC, ie: it can be used as a component decorator - The wrapped component is passed a materialStyles prop with the returned styles - The materialStyles are optimally shared between elements that of the same component that share their ThemeProvider and MaterialTheme - In the future if it becomes possible to unload styles (facebook/react-native#15899) withMaterialStyles will register these styles and unregister them when the ThemeProvider is unmounted or the materialTheme changes
|
@facebook-github-bot no-template |
|
Hey, thanks for reporting this issue! It looks like your description is missing some necessary information, or the list of reproduction steps is not complete. Can you please add all the details specified in the Issue Template? This is necessary for people to be able to understand and reproduce the issue being reported. I am going to close this, but feel free to open a new issue with the additional information provided. Thanks! See "What to Expect from Maintainers" to learn more. |
facebook-github-bot
added
the
Ran Commands
|
facepalm I included all the necessary information, the stuff in the template is irrelevant and distracting. And now a bot is telling me I have to create a new issue instead of letting me fix this one with actual details in it. ugh. |
dantman
changed the title
|
Actually this issue will require 2 PRs. A PR in react to update |
This allows objects registered with ReactNativePropRegistry to be later unregistered to free up memory, so StyleSheet can support "semi-static" styles that are mostly static but have situations they become known to be unused. It is still possible for this to work should native code starts caching props from the registry. In that case ReactNativePropRegistry.unregister would just keep a temporary blacklist of unregistered ids that would be passed to native code and the blacklist would be cleared as native removes things from its cache. The motivation for this and rationale for "semi-static" props is explained in facebook/react-native/issues/15899
|
Has there been any development on how to accomplish this sort of cleanup? |
|
No, but it was decided that rather than an unregister function it would be better for register to return objects that can be garbage collected instead of number ids. |
Motivation:
StyleSheet.createis a recommended optimization, it prevents large numbers of serialized objects from being continually passed over the js/native bridge for style updates creating a performance bottleneck. However it is only accessible to 100% static styles; if you work with anything somewhat dynamic (even if those styles will be unchanging for a long period of time and would still benefit significantly by the optimization) you cannot useStyleSheet.createbecause styles created this way are not unloaded and as values change and you replace styles you'd slowly leak memory (in js and presumably in native as well) as unused style objects stick around.This is a problem for libraries with some type of theme instance passed down/provided as props/context which "may" change but largely stay the same. Styles are contextual (one area of an app may have a dark theme and the other a light theme, or there may be more complex differences) and so any styles affected by the theme cannot be defined statically in a
StyleSheet.createcall outside the component. Which means a number of "mostly" static styles are relegated to being treated as dynamic and getting passed serialized over the js/native bridge on every render.In most of these situations some sort of provider component and/or HOC is used, in a way that it would be known when a style is no longer being used (because something about the theme has changed or the provider has been unmounted). All these components need, is a way to unload a style registered with
StyleSheet.create.Relevant code and issues:
StyleSheet registration is done through
ReactNativePropRegistry. The source does not appear to be available within the react-native but insideLibraries/Renderer/ReactNative{Stack,Fiber}-{dev,prod}.jsyou can see the compiled code. It's a relatively simple module that just has anobjectsvariable; aregisterfunction that increments a unique id, freezes the object, and sets it on the object; and agetByIDthat accepts ids and returns the objects stored inobjects.If the source were accessible, setting values on
objectsto null as part of unloading would be simple.However the whole point of StyleSheet creation is that objects are not continually passed along the js/native bridge, so presumably there is a cache on native side for style objects that also needs to be unloaded in order to prevent memory leaking. And I presume native code doesn't bother calling
getByIDall the time when it already has an object so we can't rely on unloading that way, presumably the simplest method would be forReactNativePropRegistryto have a blacklist of ids that have been unloaded which can be passed along the bridge next time some style info is passed along the bridge (the blacklist can be emptied after native is done with it to avoid it growing indefinitely).Unfortunately I do not know how these style objects registered in
ReactNativePropRegistryget across the bridge to native code. Trying variousacks and skimming the Android code I haven't been able to find the code responsible for passing styles along the bridge that would presumably be caching it. Wherever this is it feels like it's buried so deep in react-native it's inaccessible to community contributors.Real world use case:
I'm working on a library with react-native components that follow Material Design. Material Design has configurable style attributes (primary and accent color, a light and dark theme, component specific customizations, etc...) and these can be contextual (an app may have a light theme and text may be dark, but text may switch to light in the context of a solid coloured app bar; different areas of the app may have different primary colors or appBar colors; ...). So I have a theme instance that is passed down using a
MaterialThemeProvidercomponent, much likereact-redux's<Provider>or Material-UI's<MuiThemeProvider>. The provided theme is used by my library's components to determine their styles and in some cases used by consumers of the library for their styles.Because these style attributes are "provided" all the styles that are based on them (background colors, text colors, possibly padding and height, or more) end up being treated as dynamic and passed in style objects to
style. If it were possible to unload styles I would be able to provide a decorator/HOC that could be used by components (internal or part of consuming apps) to define styles using the MaterialTheme instance that the HOC along with theMaterialThemeProviderwould register withStyleSheet.createand unload when the provider is unloaded or the material theme changes.The text was updated successfully, but these errors were encountered: