clearwater icon indicating copy to clipboard operation
clearwater copied to clipboard

Router and browser reload

Open johnsusi opened this issue 9 years ago • 8 comments

I had some issues when trying to reload a page after using Router::navigate_to or when clicking a Link element.

To solve this I created a custom Router like this

class Router < Clearwater::Router

  def initialize
    super
  end

  def current_path
    path = location[:hash]
    return (path && path.start_with?('#')) ? path[1..-1] : path
  end

  def navigate_to path
    push_state "##{path}"
    set_outlets
    render_application
  end
end

So now the router keeps track of the view using fragments and location[:pathname] stays fixed.

Is there some 'official' way of doing this?

johnsusi avatar Nov 10 '15 14:11 johnsusi

Unfortunately, I haven't added support for fragment-based history at all. It's something I've been thinking about for a while, but I haven't gotten around to it. TBH, since nobody had asked, I wasn't sure it was a problem. :-) I can work on it a bit this weekend unless you'd like to send a PR before then.

The thing is, we have to somehow get Router and Link to work the same when it comes to navigation, so if Router supports fragment-based routing, Link has to support fragment-based navigation. This is the tricky part. Currently, Link's onclick doesn't go through Router#navigate_to because you don't know which instance of Router is in use by the app (you can have multiple Clearwater apps on the same page, with separate routers). I was thinking of making navigate_to a class method to fix that (since it's only changing global state, it should be fine to have it globally accessible), but I haven't checked whether that has other consequences.

Honestly, the router is the thing I'm least comfortable with code-wise. Once the tests went green while I was writing it, I basically committed it and backed away slowly. :-) I've done very little with it since then except to simplify a few things.

jgaskins avatar Nov 10 '15 20:11 jgaskins

I think the reason we had problems with this is that we are running in electron (atom-shell) as a single-page application so we never hit a server on reload. I guess it's not a problem if you can map everything below a base url to the same server-side generator.

Making navigate_to static (I'm not a ruby programmer ;-) ) sounds reasonable but perhaps we also need a flag to enable fragment based routing since it is probably not what you want in a classic client/server scenario.

For Link this is how I do it today to get it working Link.new({ href: "#some_view" }). But it would be better if that was transparent to avoid bugs.

Feel free to close this since it was more of a question that an issue :)

johnsusi avatar Nov 11 '15 09:11 johnsusi

Ahhh, cool. I wonder if we should just add a fragment-based router or make the history object inside the router (currently using window.history directly) automatically switch to a fragment-based approach if history.pushState isn't detected.

The automatic switching between fragment and pushState is something I've mulled over in my head a lot but haven't really codified yet because I haven't been sure it'd be worth it. I'd been mostly thinking that it was just IE < 10 that would need to worry about that. Not being able to use it in Electron wasn't something I'd thought of at all. Does Link work and it's just the router that's not routing properly?

I'm totally gonna keep this issue open until we can solve this. It may just be a question, but if people want to build Electron apps, I think this is all valid conversation to have. :-)

jgaskins avatar Nov 11 '15 21:11 jgaskins

Actually most things worked, but we ran into some issues after using navigate_to "/some/route". After that we could not load images anymore and it wasn't obvious (to us) why. After reading through the code and finding pushState things became clear :) (the leading / caused the documentURI to be cleared)

I think my major issue with the router is that it is an object instance on my app, but uses a global for state. That can lead to all kinds of weird behaviour that is hard to debug. Making navigate_to static at least makes that behaviour explicit.

A better approach for our application would be that Link uses the current rendering applications router instead of using href. (Or if Link is intended as a 'fixed' , introduce another component that works that way). The router would handle whatever needs to be done, be it pushState or just updating the current app model. The important part is that it is very explicit where you would handle that logic.

As a side note, when i implemented something similar in React, I passed down a route(path, ...args) prop to the components instead that they could invoke in response to events.

johnsusi avatar Nov 14 '15 12:11 johnsusi

A better approach for our application would be that Link uses the current rendering applications router instead of using href. (Or if Link is intended as a 'fixed' , introduce another component that works that way). The router would handle whatever needs to be done, be it pushState or just updating the current app model. The important part is that it is very explicit where you would handle that logic.

I think Link solves exactly the problem we need it to solve on the web side, so a new component might be in order. I've never used Electron, but I'm really interested in this, can you post a sample Clearwater+Electron app I can play around with this weekend?

jgaskins avatar Nov 14 '15 15:11 jgaskins

@johnsusi Do you have a link to a minimal/hello-world Clearwater+Electron app I could use to get started on trying to ensure the two work well together? :-)

In addition to wanting to make sure routing works well for it, since I've removed the dependency on opal-browser to try to reduce payload size for browser apps I want to make sure the new browser abstraction works well with Electron, too.

jgaskins avatar Jan 02 '16 17:01 jgaskins

@jgaskins Sorry for the late reply. I must have missed the notification.

I put up a simple project at https://github.com/johnsusi/electron-clearwater

There are some issues with it, for instance virtual-dom.js does not appear to be loading correctly but it's a start. I intent to add scripts for packaging native apps.

johnsusi avatar Apr 29 '16 12:04 johnsusi

Ah, yeah, I remember when I played with getting Electron to work with Clearwater (after you opened this issue), it seemed to have 2 JS environments: v8 running inside the app window and Node running on the back end.

I have no idea if this is accurate, I'm going off of stale memories. :-)

I remember I had to get the Clearwater code running in the app window side, so basically I was loading its file as a <script> tag inside the index.html file. I couldn't get it working otherwise. :-\ That was my one and only experience with Electron, though, so you might be able to do better with it.

I'm really glad you're experimenting with this. Let me know if there's anything I can help with! :-)

jgaskins avatar Apr 29 '16 23:04 jgaskins