Basic query loading

[WardBrian]

This is my combination of #65 and #39 on top of the new data model and react-router-dom's useSearchParams.

Jeremy's example link of

http://127.0.0.1:3000/?stan=https://gist.githubusercontent.com/magland/da3d4143276827609ec9317bb3db8b04/raw/cf48846a36de0559f96a9e4bcb5a5dcbe46383be/main.stan&data=https://gist.githubusercontent.com/magland/da3d4143276827609ec9317bb3db8b04/raw/cf48846a36de0559f96a9e4bcb5a5dcbe46383be/data.json&num_chains=6&num_warmup=1000&num_samples=1000&init_radius=2&seed=6&title=test_title

still works in this PR.

This is the first part of #50

Wishlist:

1. This added a lot of code to the ContextProvider which may be better put elsewhere
2. I noted a few places where utility functions may be useful, but some I think already exist in other PRs (I already took @jsoules' modelHasUnsavedChanges

[magland]

Aside from my two minor suggestions this looks great.

[WardBrian]

Ok, I noticed the URL wasn't clearing when you would click on a different example on the sidebar. I have a working solution, but it required the reducer to accept in a onDirty function, which I'm not entirely sure is idiomatic?

@jsoules

[jsoules]

Suggested moving some code around, a couple spurious formatting things, etc.

Probably the only substantial suggestion is that I think we might want to restructure the async stuff in the FetchRemoteAnalysis function--by eye it looks like a multi-part query string will trigger a bunch of serial fetches, awaiting between each fetch, rather than doing all the fetches in parallel. That won't be super noticeable locally, but might be perceptible for someone with a slower internet connection.

[jsoules]

I am wondering if the logic ought to be a bit more detailed here.

Specifically, it seems like dirtiness wouldn't necessarily be invoked if we're setting the sampling opts or retitling and the relevant values were not present in the query string; OR if we got a spurious call to commitFile.

Or maybe the desired behavior is just "clear the URL bar if the user breathes on anything" and we don't need to worry about this.

[jsoules]

I think this gets easier in a near future where we can easily generate a URL as well, since the guidance can just be "if you want to reshare, click the generate url button"

[WardBrian]

Or maybe the desired behavior is just "clear the URL bar if the user breathes on anything" and we don't need to worry about this.

This is the behavior I would argue for. Even editing the sampler parameters when those weren't set in the url does break the rule of "is this the page I would view if I clicked on the link again"

[jsoules]

Well, at that point it's kind of just indeterminate--if there's any part of the project that isn't set, you can't guarantee that you'll get the unset parts correct when you load, because they just aren't there. (This is more of a philosophical point right now, I'm not advocating for any particular practical decision as a result of this observation)

[WardBrian]

It would be the default value still, which is well-defined even if it isn't necessarily something we're guaranteeing will never change.

I agree something here could be more sophisticated, but for now it not being so is intentional

[jsoules]

Ahhh, okay. Confirmed that it does use the default values, not the last values.

[magland]

Tried this out.

1. I think if the user edits a file but does not save local edits, the url query should not disappear, but should stay until the save button is actually pressed. Upon reloading page, if there are unsaved edits, the user should be warned with a popup. I don't see any downsides to this, and the upside is that user won't kill the query if they accidentally type something.

2. The back button is not working as I would expect. If I make changes and the url query disappears... then I use the back button it reappears, but the content does not revert. In my opinion it should revert - this would be a value add feature. To achieve this, it will be important to restructure a bit the relationship between the state and the route. Ideally the route should be the source of truth, and the state would flow from it. Obviously when they query is missing altogether that's a separate case and the source of truth needs to shift to a different entity.

3. Ideally, if a the sampling opts were provided in the url and the user modifies one of these opts, I think it would be best to keep the url intact and to just adjust the relevant part. This goes along with the route being the source of truth for the state.

4. It will be important to not only keep track of the content of the files, but also the source of the files (when unedited)... because when a user selects "generate url" in the future, and the files were unedited from their online versions, we need to be able to repopulate that.

That's a lot to consider... maybe these adjustments should wait for a future iteration.

[WardBrian]

Item 1 is relatively simple, just changing the circumstances under which onDirty is called in the reducer. The pop up is a bit more involved, so I would prefer to leave this.

For item 2, I would need to look into the back button more but I agree on what the behavior should be in the ideal world.

I disagree on items 3 & 4. In particular, if someone sends me a link, I edit one file but not others, and I click share, I would very much like an entirely new copy to be made. That way, someone editing the sources of their original link does not make changes to mine unbeknownst to me

[magland]

Item 1 is relatively simple, just changing the circumstances under which onDirty is called in the reducer. The pop up is a bit more involved, so I would prefer to leave this.

For item 2, I would need to look into the back button more but I agree on what the behavior should be in the ideal world.

I disagree on items 3 & 4. In particular, if someone sends me a link, I edit one file but not others, and I click share, I would very much like an entirely new copy to be made. That way, someone editing the sources of their original link does not make changes to mine unbeknownst to me

Okay, let's not let 3 and 4 block this PR.

[WardBrian]

Item 2 (back button functioning as expected) seems to be a result of preserving the URL until something is dirty. Anything I did that detected when the URL parameters changed (e.g. from the back button) led to either rendering the Example buttons on the side unusable due to the current url always overwriting them, or led to an infinite loop in the renderer.

If it is implemented like I originally did way back in #39, where the URL is cleared as soon as loading is complete (the way godbolt.org works, for example), then it is "easy".

See diff here: query-url-loading...query-url-loading-back-button

This also avoids the onDirty trickery in the reducer, so I prefer it, but it is a different feature set than the one we had agreed on a couple weeks ago.

I would also be fine revisiting this in a later PR, if we'd prefer that. @magland @jsoules

[magland]

I would also be fine revisiting this in a later PR, if we'd prefer that. @magland @jsoules

I agree, this is quite tricky. I'd vote to leave it as is for now (where back button restores query but does not reload things)

[jsoules]

I'm inclined toward just clearing the URL bar if it's causing problems, but we can worry about that later.

I'll merge this as is for now.