Skip to content
This repository has been archived by the owner on Oct 10, 2024. It is now read-only.

Create sessions only when needed #70

Closed
wants to merge 1 commit into from
Closed

Conversation

joemasilotti
Copy link
Owner

This PR moves the two Session properties to be lazy evaluated. Before, whenever a TurboNavigator instance was created the two sessions would be created right away. Which meant that two web views were created.

I think this is fine with a single TurboNavigator instance. But when working with tabs you could create five of these when the app launches. I don't like the idea of creating ten web views every time the app launches.

@joemasilotti
Copy link
Owner Author

@olivaresf, do you see this causing any issues with the custom initializer we added in #48?

@olivaresf
Copy link
Contributor

I don't see this causing an issue with the init; though it's interesting that I feel they're related. I wouldn't be entirely worried with creating 10 WebView instances 🤔. Have you measured the impact it would have on initialization?

In any case, the initializer allows the caller to decide when (and how) to instantiate a Session, then pass that along to TurboNavigator. That also solves the issue you're bringing up, which is that the caller can decide to instantiate each session as each tab appears.

Maybe we should reconsider session ownership of TurboNavigator somehow?

@joemasilotti
Copy link
Owner Author

Have you measured the impact it would have on initialization?

I've profiled it and watched the memory climb. But I haven't run into any specific issues caused by using so much memory.

That also solves the issue you're bringing up, which is that the caller can decide to instantiate each session as each tab appears.

Do we want folks to customize the Session directly? I feel like customizations at this level of abstraction (Turbo Navigator vs. the current iteration of turbo-ios) shouldn't deal with Session. Yes, web view customizations are still needed for Strada but maybe that's it?

@olivaresf
Copy link
Contributor

Do we want folks to customize the Session directly?

Probably not? I think passing a session should be simple enough without customization.

@joemasilotti
Copy link
Owner Author

Closing in favor of #74.

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

Successfully merging this pull request may close these issues.

2 participants