Glimpse / Home

Project Glimpse: Node Edition - Spend less time debugging and more time developing.
http://node.getglimpse.com
Other
252 stars 9 forks source link

[Client] Deep links don't appear to be reusable (?) #20

Closed lostintangent closed 7 years ago

lostintangent commented 8 years ago

Repro Steps:

  1. Select a request entry within the Glimpse client
  2. Copy the current URL from the browser's address bar
  3. Open a new browser tab and paste the URL into it

Expected: To be navigated to the exact request that the URL points to Actual: I get the following error, and have to navigate back to /glimpse/client and find the request again (in Chrome).

Cannot GET /glimpse/client/requests/fcbd12807b6d11e6bf4e5b3990f7f65f/request?requestAxis=headers&responseAxis=headers

Note: I can repro this issue by simply refreshing the browser tab when a request is selected, so the bug doesn't appear to be restricted to a new tab.

nikmd23 commented 8 years ago

Thanks for your report!

philliphoff commented 8 years ago

There's (at least) a couple of issues here:

  1. When the Client is first launched, it hits the Server client resource (ClientEmbeddedResource, at /glimpse/client) which serves up the raw files. Once the client is running, react-router then controls the browser history (i.e. what's shown in the location bar) without making subsequent requests of the server.

    When you refresh the browser window or use that URL in another window, the server will be hit again, but using the complete react-router path (e.g. /glimpse/client/requests/.../request?requestAxis=headers&responseAxis=headers). The server doesn't know what to do with this path (as it doesn't correspond to an existing directory/file hierarchy) and so returns a 404.

    This appears to be a known complication with purely client-side react-router applications served. The general answer seems to be adding routing such that the server falls back to serving index.html whenever a matching static file is not found (which effectively ignores any path resulting from react-router). However, this approach generally makes the assumption that the application is served at the root of the server and that any dependencies of the application (e.g. css, js, png, etc.) are specified within the application using absolute paths. Otherwise, if using relative paths, the server will get asked to serve up that non-HTML content with the react-router paths and fail. We expose the Glimpse client at an arbitrary, non-root path (/glimpse/client). Using this approach would mean embedding that path throughout the client, which is less than ideal and could interfere with running the client stand-alone.

    An alternative is to use hashHistory rather than browserHistory as the store of navigation history. This causes react-router navigation to be based on anchor tags (i.e. hashes). The advantage is that a refresh or copy & paste will always fetch the root server endpoint (because hash navigation is done strictly client-side). The disadvantage is the use of somewhat awkward looking hashes. The bigger disadvantage is that we need to pass parameters to the client via query parameters (e.g. the metadata URI). As query parameters are separate from hash parameters, they will always show up in the location.

    My current thinking is to take a modified approach where the EmbeddedClientResource serves up the static files directly (i.e. doesn't rely on Express' middleware) by effectively ignoring any paths generated by react-router. This means that client assets can still be specified relatively, but it also means that client assets must all exist a single, root folder as we cannot determine where the split between react-router path and directory/file path exists. (This happens to be the case today, but it does add a constraint on the client.)

  2. Once the server is capable of serving up the client in at any arbitrary route, we run into the next issue: the client makes assumptions about requests already having been fetched prior to any given request being selected. We need to do some refactoring of the client to add robustness when it's asked to launch with an arbitrary request selected, then likely to issue the fetch and wait and auto-refresh its views when the fetch completes (or fails).
avanderhoorn commented 8 years ago

This means that client assets can still be specified relatively, but it also means that client assets must all exist a single, root folder as we cannot determine where the split between react-router path and directory/file path exists. (This happens to be the case today, but it does add a constraint on the client.)

I'm onboard with updating the EmbeddedClientResource to server up the files directly, but I'm not sure what you mean by the above. Do you mean that they the assets need to sit in the same folder as the bundle.js instead of a sub folder like ./assets or something?

I also agree with 1 needing to be sorted before 2, it was the same conclusion I came to. If you want to slit these up into separate issues and treat this issue as the "investigation" task feel free (just update the estimate to reflect that).

philliphoff commented 8 years ago

I'm onboard with updating the EmbeddedClientResource to server up the files directly, but I'm not sure what you mean by the above. Do you mean that they the assets need to sit in the same folder as the bundle.js instead of a sub folder like ./assets or something?

Yes, all client assets would need to exist within the same, single folder. This is because we get requests for /glimpse/client/requests/abcd/request/bundle.js (because we earlier served up index.html at path /glimpse/client/requests/abcd/requests in order to preserve the current react-router route). I don't think there's a reasonable way to discern whether /requests/abcd represents react-router path or filesystem path (or some combination thereof). So, we have to constrain the client to a single folder so we can just grab bundle.js from the end of the path, and serve that up from the known client folder.