Interoperability and wasmJs bugfixes #412
Conversation
…aks and performance degradation after large number of recompositions
| @@ -41,20 +59,34 @@ public fun Router.route( | |||
| CompositionLocalProvider(RouterCompositionLocal provides this) { | |||
| val rawPath by getPath(initRoute) | |||
| val path = Path.from(rawPath) | |||
| val node = remember(path) { RouteBuilder(path.path, path) } | |||
| val node = remember(path) { | |||
| RouteBuilder.routeBuilderCache.getOrPut("${this.hashCode()} ${path.path} $path") { | |||
There was a problem hiding this comment.
Why can't we put all parameters to remember? Remember uses its own cache.
There was a problem hiding this comment.
Remember only works with a recomposition. The routing changes the composition (route calls are leaving and entering the composition when navigating) and remember doesn't work here.
There was a problem hiding this comment.
Do you have any samples causing the memory leaks (or a reproducer)? I do understand the cause, we create a new RouteBuilder during each composition, but ideally the GC/Compose should be able to remove the object.
| @@ -24,7 +24,7 @@ public fun BrowserRouter( | |||
| initPath: String, | |||
| routeBuilder: @Composable RouteBuilder.() -> Unit | |||
| ) { | |||
| BrowserRouter().route(initPath, routeBuilder) | |||
| Router.internalGlobalRouter = BrowserRouter().apply { route(initPath, routeBuilder) } | |||
There was a problem hiding this comment.
What's the use-case/code for setting this variable? Can't the function just return the Router instead?
There was a problem hiding this comment.
It could. I've implemented it this way just to simplify the client code and to be consistent with Router.current. In my applications I just use Router.global when Router.current can't be called.
There was a problem hiding this comment.
What do you mean with can't be called? Because Router.current is @Composable or because there is no value set?
There was a problem hiding this comment.
It can't be called from any non-composable functions like effects, event handlers, view model methods or some global utility functions.
|
BTW I will cherry pick your wasm fixes in another PR. |
This PR fixes wasmJs navigation as well as some memory leaks. It also makes the library more open and easier to extend (I'm using it in my Kilua framework).