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

Clarity on establishing initial state #185

Open
bahrus opened this issue Nov 12, 2021 · 3 comments
Open

Clarity on establishing initial state #185

bahrus opened this issue Nov 12, 2021 · 3 comments

Comments

@bahrus
Copy link

bahrus commented Nov 12, 2021

The documentation states:

We believe that in the majority of cases, single-page apps will be best served by updating their state via appHistory.navigate({ state: newState }), which goes through the navigate event. That is, coupling state updates with navigations, which are handled by centralized router code. This is generally superior to the classic history API's model, where state (and URL) updates are done in a way disconnected from navigation, using history.replaceState().

It then goes on to describe the "one type of case where the navigation-centric model doesn't work well".

However, is it not considered a valid use case to call updateCurrent when initializing a session, based on the url, assuming there is no history? For example, a user opening the url for the first time, based on parsing the url using URLPattern, for example?

I see there's another issue asking if any events should fire on page load. I would think that if use of updateCurrent is not considered a valid use case for initializing state, then yes, an event should fire on page load, marked clearly in some way to the effect "new session started". I'm just not gleaning that clearly either from the current Readme of this repo, nor from the main spec's. Apologies in advance if I missed it.

Also, I'm seeing behavior that doesn't match what I would have expected, based on how history.setState behaves (which perhaps I need to understand a new mindset based on the comment above).

If I use updateCurrent and set some state values, then refresh the page, they seem to be gone. Maybe I'm not utilizing the api properly, or maybe this feature isn't incorporated yet? Or is this an old api vs new api mindset I need to adjust to?

Again, apologies for raising this as an issue when it is probably me fumbling around with a new api -- I would feel more comfortable raising these questions if the "discussion" feature of github were enabled for this repo (but that may just be me).

@domenic
Copy link
Collaborator

domenic commented Nov 12, 2021

This is a fair point, and I suspect you're right.

I think maybe I wrote the text from the perspective of an idealized model. Something like: whenever a user navigate directly to a page by URL, then all the important state is captured in the URL itself. So there shouldn't be a need for anything to be in the state object. You only need to update the state object when the user enters non-default states, or when you want to traverse to another state/URL (using navigate()).

But I imagine in practice, that division doesn't entirely hold. If you have some concrete examples to share, that could help me in making the text more realistic.

an event should fire on page load, marked clearly in some way to the effect "new session started"

Well, several events already fire on page load, e.g. load and DOMContentLoaded among others. Also, anything you place in <script> tags runs immediately. So I don't think it'd be very hard to initialize the appropriate state on page load using what exists today; a new event doesn't seem necessary.

If I use updateCurrent and set some state values, then refresh the page, they seem to be gone.

That does not match what I observe. Using Chrome Canary 98.0.4695.0 with Experimental Web Platform features turned on in chrome://flags:

  • Navigate to https://example.com/ and open the console
  • Type appHistory.updateCurrent({ state: "foo" })
  • Type appHistory.current.getState(). As expected it returns "foo"
  • Reload the page.
  • Type appHistory.current.getState(). As it expected it still returns "foo".

How exactly are you seeing the state disappear?

Again, apologies for raising this as an issue when it is probably me fumbling around with a new api

No problem at all!

@bahrus
Copy link
Author

bahrus commented Nov 12, 2021

How exactly are you seeing the state disappear?

I stand corrected. Dumb mistake on my part. Glad to see that behavior retained, and thanks for checking.

So I don't think it'd be very hard to initialize the appropriate state on page load using what exists today; a new event doesn't seem necessary.

I agree -- the wording of the documentation suggested to me it might be considered an anti-pattern to initialize state outside of an officially provided navigate function or event. That appears to be me reading too much in that paragraph.

A quick survey of popular sites I could find that use history.state (twitter, github) indicates a mixed bag as far as initializing history.state on page load. Twitter does, github doesn't.

. Something like: whenever a user navigate directly to a page by URL, then all the important state is captured in the URL itself. So there shouldn't be a need for anything to be in the state object

As far as finding parts of or all of the url in history, twitter puts the full url in history.state.

If I open this page directly in a new browser tab:

https://www.msn.com/en-us/news/coronavirus?ocid=msedgntp

I see the following put into history.state on page load:

{
    "display": "Coronavirus",
    "id": "news-coronavirus",
    "renderInfo": {
        "renderType": 3,
        "externalUrl": "https://www.msn.com/en-us/news/coronavirus",
        "path": "news/coronavirus",
        "usePathNameAsIs": true,
        "context": {
            "feedId": "Y_46cba2b4-26fd-492e-a1c0-41ec9917c10b"
        },
        "experienceConfigRef": {
            "instanceId": "",
            "configRef": {
                "experienceType": "River",
                "instanceSrc": "_hub-news-coronavirus-river-index-BB10Xdfl",
                "sharedNs": "msn-ns"
            }
        }
    },
    "pageTitle": "Coronavirus (COVID-19)",
    "verticalKey": "news",
    "categoryKey": "coronavirus",
    "destinationUrl": "https://www.msn.com/en-us/news/coronavirus?ocid=msedgntp",
    "parentId": "news",
    "telemetryMetadata": {
        "n": "Coronavirus",
        "a": "click",
        "b": 1,
        "d": "https://www.msn.com/en-us/news/coronavirus?ocid=msedgntp",
        "f.i": "news-coronavirus",
        "f.n": "Coronavirus",
        "f.t": "category filter",
        "c.hl": "Coronavirus"
    }
}

Hopefully no concerns on your part doing this with the new appHistory api?

@domenic
Copy link
Collaborator

domenic commented Nov 12, 2021

No concerns! It sounds like I should probably update the documentation to mention that is another reasonable use case for updateCurrent().

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