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

Consider adding a general entrieschange event #205

Open
domenic opened this issue Mar 14, 2022 · 1 comment
Open

Consider adding a general entrieschange event #205

domenic opened this issue Mar 14, 2022 · 1 comment
Labels
addition A proposed addition which could be added later without impacting the rest of the API

Comments

@domenic
Copy link
Collaborator

domenic commented Mar 14, 2022

Continued from #148:

It might be useful to have a general entrieschange event for whenever the result of navigation.entries() would return a new value. In addition to the cases where currententrychange and dispose fire currently, this would also fire if new entries grow in the back of the list due to #45, or if we allow other modifications like #9.

The use cases this would enable, in addition to the above, are:

The event object here could contain e.g. addedEntries, removedEntries, updatedEntries. Or we could have the developer save the old entries list and compare manually.

@domenic domenic added the addition A proposed addition which could be added later without impacting the rest of the API label Mar 14, 2022
@fabiancook
Copy link

fabiancook commented Sep 25, 2022

What entries end up in updated? Would it only ever have 0 or 1 items?

  • navigate -> added + removed (if traverse back then navigate), or updated if using replace
  • reload -> updated
  • traverseTo -> none
  • back -> none
  • forward -> none
  • updateCurrentEntry -> updated
  • disposed -> removed

I did think back/traverseTo should have removed, but, the entries don't change, none are removed, just the currentEntry?

It would seem fair to let someone who is adding an event listener to also maintain their own local scoped state though.
async function *entryChanges(compareStateFn) {
  let snapshot = navigation.entries(),
    stateSnapshot = new Map(snapshot.map(entry => [entry.key, entry.getState()]));
  
  while (true) {
    await new Promise(resolve =>  navigation.addEventListener("entrieschange", resolve, { once: true }));
    
    const next = navigation.entries();
    const removed = snapshot.filter(entry => !next.includes(entry));
    const added = next.filter(entry => !snapshot.includes(entry));
    const nextStateSnapshot = new Map(next.map(entry => [entry.key, entry.getState()]));
    const updated = next.filter(entry => !stateSnapshot.has(entry.key) || compareStateFn(stateSnapshot.get(entry.key), nextStateSnapshot.get(entry.key)));

    yield { removed, added, updated };
    
    snapshot = next;
    stateSnapshot = nextStateSnapshot;
    
  }
  
}

Maybe if the entires list gets long this will become a bit more costly.. which might be a case for including these... I would imagine it would be easier for the navigation api to make sense of where entries are added/removed/updated, vs being calculated by a user every time.

currententrychange uses the name from for its related entry, instead of fromEntry

Could entrieschange use added, removed, updated instead?

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