Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle Browser.Navigation.Key that is not at the root of the app model #10

Closed
Punie opened this issue Sep 6, 2018 · 1 comment
Closed
Assignees

Comments

@Punie
Copy link

Punie commented Sep 6, 2018

Based on the discussion we add on Slack yesterday about the feasibility of such an improvement, I'm taking the liberty of opening this issue to easily track progress as well as coordinate efforts should some benevolent soul try and take a shot at this.

Some relevant information from @klazuka :

It can definitely be fixed (PRs welcome). At the time, I just wanted to get something out the door, and I hadn’t yet seen changes to rtfeldman’s SPA example. So it seemed like a reasonable compromise. But now that that example is out there, it’s less tenable of a solution.
The core problem is that Browser.Navigation.Key is not plain-old data: it’s actually a function that closes over the Elm runtime’s sendToApp function. So unlike all of the other data in your model, the navigation key cannot be reused in the new app. So we need to be able to find it in the old model and swap it out with the new app’s key.
My implementation just looks one level deep in your model. We could certainly crawl the model recursively to try to find all instances of the nav key and update them in the new app’s model.

Unless I'm mistaken, the relevant code should be around here : https://github.com/klazuka/elm-hot/blob/master/resources/hmr.js#L303-L325

@klazuka klazuka changed the title Improvement: lift requirement to have Browser.Navigation.Key at the root of the Model Handle Browser.Navigation.Key that is not at the root of the app model Sep 6, 2018
@klazuka
Copy link
Owner

klazuka commented Sep 6, 2018

I'm going to try to fix some of the other bugs today. If there's time, I'll do this one today or tomorrow.

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

No branches or pull requests

2 participants