unabridged / motion

Reactive frontend UI components for Rails in pure Ruby
https://github.com/unabridged/motion
MIT License
697 stars 19 forks source link

Domain lost after page change #75

Closed HashNotAdam closed 3 years ago

HashNotAdam commented 3 years ago

When working with links that require the current domain, the domain is correctly loaded on the first load but will be replaced with http://example.org upon any page change.

This can be reproduced simply by adding <%= root_url %> to a page and using a periodic timer to re-render the page.

While absolute links are uncommon, they are used by ActiveStorage's direct upload. I have been able to avoid this issue by storing the URL to an instance variable in the controller and passing that value into the component so that it isn't being regenerated on page change.

latortuga commented 3 years ago

@HashNotAdam thanks for the bug report, is there any chance you could provide a working example application that exhibits this bug? You can use https://github.com/unabridged/motion-demos as an example starting point if it is helpful.

HashNotAdam commented 3 years ago

@latortuga https://github.com/HashNotAdam/motion-issue-75

clottman commented 3 years ago

@HashNotAdam Do you see the issue behave the same way in the demo repo you provided? I'm looking at it now and it actually isn't updating the URL at all; rather, it throws an error while attempting to re-render after each tick.

[CurrentDomainComponent:10340] Processed periodic timer tick (in less than 0.1ms)
  Rendering CurrentDomainComponent
  Rendered CurrentDomainComponent (Duration: 0.5ms | Allocations: 411)
[CurrentDomainComponent:10340] An error occurred while rendering the component:
    ActionController::UrlGenerationError: No route matches {}

I'm assuming that the domain being set to example.com is coming from somewhere inside the app that you're working on, which isn't happening in the demo app, and so it's behaving in a slightly different way. Does that sound plausible?

clottman commented 3 years ago

After digging into this a bit more, and looking at the internals of how url_for and related methods work, the team thinks that the behavior you're seeing should be the expected behavior. The workarounds you're using now are what we'd expect other users in this same position to use.

The reason, and something we should make more clear in the documentation, is that when you are using motions, you lose access to the request object after the initial render, and instead just have the websocket events sending data related to your component only back and forth. So, if you want to keep anything from the initial request, like the host the request came from to use in absolute urls, you should cache that yourself as properties on the view.

Thanks so much for submitting this issue @HashNotAdam. It resulted in some good conversations about how motion should work, and will ultimately result in documentation updates that make that intended behavior more clear.