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

Add info option to updateCurrent, provide with currentChange event #186

Open
bahrus opened this issue Nov 13, 2021 · 4 comments
Open

Add info option to updateCurrent, provide with currentChange event #186

bahrus opened this issue Nov 13, 2021 · 4 comments
Labels
addition A proposed addition which could be added later without impacting the rest of the API

Comments

@bahrus
Copy link

bahrus commented Nov 13, 2021

Proposal:

export interface AppHistoryUpdateCurrentOptions {
    state: unknown;
}

should allow for an info option:

export interface AppHistoryUpdateCurrentOptions {
    state: unknown;
    info: unknown
}

Correspondingly, the change event should provide that info option in the event:

export declare class AppHistoryCurrentChangeEvent extends Event {
    constructor(type: string, eventInit: AppHistoryCurrentChangeEventInit);

    readonly navigationType: AppHistoryNavigationType | null;
    readonly from: AppHistoryEntry;
    readonly info: unknown
}

Some use cases that come to mind:

  • Be able to indicate which portion of the state changed, for better pinpointing reactive updates
  • Provide information about what prompted the change (for example expand a details tag vs closing it)
@bahrus bahrus changed the title Add info option to updateCurrent, provide with curre Add info option to updateCurrent, provide with currentChange event Nov 13, 2021
@domenic
Copy link
Collaborator

domenic commented Nov 15, 2021

I'm a bit hesitant to add more semantics here, as this changes updateCurrent() from being about updating the state/URL, to being an arbitrary communications channel between two parts of the app. Although such communications channels are useful, it's not clear whether this is the exact right place to slot that.

Can you give an example, preferably from a real app you're building, as to how this would be helpful?

@domenic domenic added the addition A proposed addition which could be added later without impacting the rest of the API label Nov 15, 2021
@bahrus
Copy link
Author

bahrus commented Nov 15, 2021

Can you give an example, preferably from a real app you're building, as to how this would be helpful?

A fair question. Let me first confess that I'm not currently building this now. Nor have I built killer apps like Uber or anything. And our applications are built in an (internal) environment where the target audience has lots of bandwidth and pc firepower. Our backend is the bottleneck.

In my organization, I have certainly worked on plenty of applications like what I'll describe below.

We didn't necessarily rely on history.state to accomplish this. In fact, only recently in a tiny number of projects (ones I was involved with due to my adventurous / reckless spirit) did we experiment with history.state at all,
which in my view made the application much easier to maintain, for reasons described below.

But using history.state for the use case below would be far more tempting, and a stronger case to use it, if history.state was more capable.

I think the appHistory initiative is definitely a significant step up, but this would perhaps be the crowning jewel (okay, maybe that's too strong).

My organization has clients and products and employees and revenue (unique, aren't we? :-) ).

Often times the application requirements are established so that the user goes into client mode vs products versus teams versus revenue via a left menu, and those become the basis for our key routes.

In each of our routes, we have "global filters" which provide rather large tree like structures, in order to restrict the list of clients and/or products and/or teams (let's say to slice and dice revenues).

In our main view area, we have lots of grids and charts, which can be drilled into multiple levels down, with modal popups appearing sometimes to edit data.

Requirements for supporting the browser back button, refresh and also user supplied back buttons, and breadcrumb histories are almost always there as well. It goes without saying that the global filter settings should be retained when traversing between routes.

Knowing the values of the global filters deep into the DOM hierarchy, and updating only the required parts of the UI when the global filters change, is thus quite critical. So there's that.

So this is probably the classic "prop drilling" issue, which of course there have been lots of proprietary solutions coming and going into and out of favor for -- flux, redux, context api, etc. In fact, we have tended to use redux for this purpose,
(which no one really likes in my group, but we all use because, that's what we're supposed to use isn't it?) Hence my piqued interest in history.state.

Then there's the question of restoring the filters when we come back to previous routes.

If we use a router that keeps views "alive", like Vue's keep-alive router (or Polymer did this too), the views that were already opened, with the filter selections, are preserved based on the in-memory UI. Easy peasy!

However, let's suppose we use a SPA that is more like what 90% of applications use, and the previous view is dumped.

So now we seem to be rather hard pressed to instantiate separate objects to store this state, for when the user returns, so something like redux seems like the natural fit.

Which is a shame, because it seems to me history.state is a better place to store the filters. If the reason for dumping old routes is better use of memory, if the state of inactive routes is stored on disk (is that an invalid assumption with appHistory?),
wouldn't that be better? (We can't put clients in any permanent storage like IDB, btw).

So based on that reasoning, let's say we store these filters in appHistory. Since appHistory is accessible globally within the window / iframe, and since there's some nice events we can subscribe to to be notified when currentState changes,
deep drilling of props problem is solved. The only misgiving is that we don't really have a clean way of passing what changed, which other state libraries like MobX provide.

I'm sure it is not the intention of the appHistory proposal to compete with solutions like redux and MobX, which I'm sure can fulfill much more than our simple requirements.

But if this proposal/issue is implemented, I think there is a problem space it would solve (mine, basically), with the help, perhaps, of a minimal helper library, and a smaller learning curve.

Am I way off base? :-)

@domenic
Copy link
Collaborator

domenic commented Nov 16, 2021

Thanks for the long description of such apps :). If I'm understanding correctly, the main point per

The only misgiving is that we don't really have a clean way of passing what changed, which other state libraries like MobX provide.

is that instead of code like this

let previousState = appHistory.current.getState();

appHistory.addEventListener("currentchange", () => {
  const newState = appHistory.current.getState();
  const whatChanged = compareStateValues(newState, previousState);
  previouState = newState;

  doImportantStuff(newState, whatChanged);  
});

which requires writing a compareStateValues() function, it would be easier if you could do something like this:

appHistory.addEventListener("currentchange", event => {
  const newState = appHistory.current.getState();
  const whatChanged = event.info;

  doImportantStuff(newState, whatChanged);  
});

I'm not sure this would work, since for navigations (i.e., all the cases except the specific case pointed out in setting the current entry's state without navigating, such as history traversals by the user) event.info would not contain any information. event.info would only be set if you explicitly called appHistory.updateCurrent({ state: newState, info: whatChanged }). So you'd have to have some way of calculating what changed anyway, for the navigation cases.

This is partially why we say:

It is best not to use this event as part of the main routing flow of the application, which updates the main content area in response to URL changes. For that, use the navigate event, which provides interception and cancelation support.

@bahrus
Copy link
Author

bahrus commented Nov 17, 2021

Yes, I think you have correctly summarized what I am after, with one possible area of disagreement.

The documentation states:

Use appHistory.updateCurrent({ state: newState }), if your state update is meant to capture something that's already happened to the page.

And you state: So you'd have to have some way of calculating what changed anyway, for the navigation cases.

As I see it, in the context of an application like I'm describing where the router dumps previous navigational views, there are big navigational changes (drilldowns, breadcrumb links, menu selections), where it is acceptable to redraw the main area from scratch (or based on some cache of data), so knowing what has changed isn't needed. The fact that I won't know what changed, then, doesn't concern me. I just need to know what the last state of the checkbox filters was.

But now, within the context of one navigation, there are "micro" changes where different tree filter checkboxes are checked / unchecked, and users want to immediately see what impact that has on the data as they check / uncheck.

When the checkbox changes from unchecked to checked, that's something that's already happened to the page. I now want to a) update state so that if I come back later to this view via a major navigation, I know what the last selections are. In that scenario, I don't need to know what changed. But b), while that view is open, apply the minimal changes needed to the elements in that view.

Updating the existing charts / grids in the existing view would benefit from knowing what changed, for more precisely adjusting / calculating the impact on the data, giving the user real-time updates as they check / uncheck. Similar to filtering / sorting a grid, column by column. When specifying an additional column that is filtered, we can take the existing filtered list, and just check the new column filter, without rechecking all the other filters applied. Yes, it is updating "the main content area" but not in response to URL changes, and the updates are data related, not structural. textContents only change, sizes of pie chart wedges, etc. But nothing structurally moves around, appears out of thin air, disappears.

Maybe that's a blurry line, but it feels qualitatively different to me. Am I wrong?

It is kind of like what I was asking in another issue about supporting "document editing history" which you indicated is out of scope. I'm not asking for a document history, here, just a way of communicating little micro changes. In fact a userland document history engine could be built on top of what I'm proposing, I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition A proposed addition which could be added later without impacting the rest of the API
Projects
None yet
Development

No branches or pull requests

2 participants