Polyfill

[qwtel]

FWIW I've implemented the remainder of this polyfill, specifically History API integration, over at qwtel/navigation.

I've also changed a bunch of stuff to my personal tastes and it has some rough edges, so this isn't a straight forward PR, but feel free to take a look. EDIT: The bulk of the work is located in get-polyfill.ts.

Also, thanks for going through the trouble of writing this. An upcoming version of https://sqliteviewer.app will use it.

[fabiancook]

Hi, I'll be keen to review the integration and bring it into this module.

The core of the functionality is implemented here, the history integration being the final part.

[fabiancook]

I think it would be great if you opened the PR up and let's see the diff, then we can work on a plan on how to merge them together :)

[fabiancook]

Looking through the changes, I definitely think that it is all useful stuff and shouldn't need a fork going forward. The changes are very useful and would contribute nicely!

Will be awesome to see if we can get the final web platform tests passing with this!

The getNavigation function was originally be an entrypoint into the various ways we could get navigation going, with a polyfill being one of them. Though browsers aren't the only target of this module.

I think all the code for the polyfill can be moved into another file (a file actually named get-polyfill.ts, which get-navigation.ts would use)

The settings, instead of being hard coded, because we are messing with the global scope anyway, can be pulled from the global scope as well. This would make it easier for setup to happen. Another way is to read a json object from a script element defined in the document if we are in a browser! For node/deno/bun we would use from env in the future if really needed.

With the external dependency for structured clones, I found that if we can do best effort without the dependency, and then bringing it in if its available. You can see me doing that for other things like uuid here

Cheers!

[qwtel]

Will be awesome to see if we can get the final web platform tests passing with this!

Would be nice, but not too optimistic. I didn't implement this per spec, just trying to match chrome behavior for the subset I've needed. Also, apparently there are some edge case relating to how it interacts with iframes -- I haven't considerd this at all.

I think it would be great if you opened the PR up and let's see the diff, then we can work on a plan on how to merge them together :)

I've opned a PR, but as I've said there are unrelated changes in there. I'll see if I can clean this up later this week.

The hard-coded settings, the placement in get-polyfill, etc. it could all be improved.

Reading settings from global object might not be so bad. I initially thought about turning the polyfill into a function call and pass the settings that way, e.g. polyfillNavigation({ /* settings */), but never got around to it. Also, I've never liked that pattern much myself.

[loganvolkers]

There is some argument for both a polyfill and a ponyfill: https://github.com/sindresorhus/ponyfill

One will automatically change global scope, and the other will not. Given the state of APIs it might not be possible to achieve a ponyfill, but if a ponyfill is achieveable, then a polyfill is usually just some light code on top of it.

The settings, instead of being hard coded, because we are messing with the global scope anyway, can be pulled from the global scope as well. This would make it easier for setup to happen. Another way is to read a json object from a script element defined in the document if we are in a browser! For node/deno/bun we would use from env in the future if really needed.

I think that there is a big difference between polyfilling a global and introducing new globals. I think that introducing new globals is counter to both the polyfill and ponyfill philosophies. Go for the callable polyfill instead, e.g. polyfillNavigation({ /* settings */)

[fabiancook]

FYI the original intention was for navigation never to be added to the global scope by default, unless it was explicitly imported using @virtualstate/navigation/polyfill

In the feature/polyfill branch I have worked on the provided PR a bit, and added a few more patterns which I described on the PR, but here is some snippets:

const navigation = new Navigation(),
  history = new NavigationHistory({ navigation }),
  window = new EventTarget(),
  document = new EventTarget();

window.document = document;
window.history = history;

// Gets a new evented polyfill using the window, doesn't event based on the global window or history
const newNavigation = getPolyfill({ window })

// Applies an already existing polyfill to the global scope
applyPolyfill({ navigation })

// Applies a new polyfill to the global scope, based on the history in the window
applyPolyfill({ window })

// Applies a new polyfill to the global scope, based on the history in the global window
applyPolyfill()

// Imports all the needed polyfill code, and calls applyPolyfill with the navigation returned from the default getNavigation
await import("@virtualstate/navigation/polyfill");

Where no options are given, there is a frozen DEFAULT_POLYFILL_OPTIONS defined

export const DEFAULT_POLYFILL_OPTIONS: NavigationPolyfillOptions = Object.freeze({
  persist: true,
  persistState: true,
  history: true,
  limit: 50,
  patch: true,
  interceptEvents: true
})

Another add I have is that if there is no window.history, we can still polyfill history as well, as I made a reverse polyfill for history, built on top of navigation

So if you do...

const window: EventTarget & { history?: History } = new EventTarget();
applyPolyfill({ window });
const { history } = window;

history should work as expected.

I moved the getState pattern so that the entire function can be customised through passing a function through to the constructor of navigation

const navigation = new Navigation({
  getState({ key }) {
    const maybe = sessionStorage.getItem(this.id);
    if (!maybe) return undefined; // Or can return null here
    return JSON.parse(maybe);
  }
})

It is used in the place that the polyfill code was needed to initiate state:

  getState<ST extends S>(): ST;
  getState(): S;
  getState(): unknown {
    let state = this.#state;

    if (!state) {
      state = this.#state = this[NavigationGetState]();
    }

This then allows for another direction that can be externally configured

new Navigation({
  getState({ key }) {
    const maybe = sessionStorage.getItem(this.id);
    if (!maybe) return undefined; // Or can return null here
    return JSON.parse(maybe);
  },
  setState(entry) {
    const state = entry.getState();
    if (state) {
      sessionStorage.setItem(entry.key, JSON.stringify(state)); 
    } else {
      sessionStorage.removeItem(entry.key)
    }
  }
})

You could write your own setState using currententrychange, but this here is just first class support for it, and makes sense seeing it alongside getState

Theres another part to this too... what happens when we dispose entries from the list (I saw this was in yourcurrententrychange event handler too)

https://github.com/WICG/navigation-api#notifications-on-entry-disposal

So it might warrant one more function. Which would be called the same time as navigation.currentEntry.addEventListener("dispose", fn) would be called. So, I would call it, disposeState

new Navigation({
  getState({ key }) {
    const maybe = sessionStorage.getItem(this.id);
    if (!maybe) return undefined; // Or can return null here
    return JSON.parse(maybe);
  },
  setState(entry) {
    const state = entry.getState();
    if (state) {
      sessionStorage.setItem(entry.key, JSON.stringify(state)); 
    } else {
      sessionStorage.removeItem(entry.key);
    }
  },
  disposeState({ key }) {
    sessionStorage.removeItem(entry.key);
  }
})

I've then expanded the options for the constructor further so you can construct it how you need out of the box:

new Navigation({
  currentKey: "two",
  entries: [
    {
      key: v4()
    },
    {
      key: v4(),
      url: "/1"
    },
    {
      key: "two",
      url: "/2"
    },
    {
      key: v4(),
      url: "/3"
    },
  ],
  getState({ key }) {
    const maybe = sessionStorage.getItem(key);
    if (!maybe) return undefined; // Or can return null here
    return JSON.parse(maybe);
  },
  setState(entry) {
    const state = entry.getState();
    if (state) {
      sessionStorage.setItem(entry.key, JSON.stringify(state)); 
    } else {
      sessionStorage.removeItem(entry.key);
    }
  },
  disposeState({ key }) {
    sessionStorage.removeItem(entry.key);
  }
})

[fabiancook]

You'll note I didn't do anything specifically to persist entries.

The default pattern mostly has a way to deal with it, and when it persists inside setState in the polyfill, it persists the entries too, then stores the current entry id on the history.state, so if the page is reloaded or popped, it is able to fetch the list back from there.

[qwtel]

I like the refactoring, it's all very clean and looks good. Unfortunately multiple pieces seem to be broken now:

Just accessing the state on history causes infinite loop:

Intercept doesn't seem to be calling prevent default, clicking <a> navigates the page

navigation.navigate does not seem to call pushState, etc.

I've tried debugging it for a while, but to no avail, and I'm out of time now 😕 . Sorry I can't be of more help here.

[fabiancook]

Thanks for the detail, I'm going to merge it together and then see what up. I'll try replicate it with the tests and go from there. The general idea seems to working together, I'll debug it step by step.

[fabiancook]

RE maximum call stack, I realised that I was no longer using the original getter for history.state. I did two things for it.

history.state I have replicated to be history.originalState now, which will be the native state given.

history.state, polyfilled, will be tied directly to the state of navigation.currentEntry.getState() now, which will eventually lead to history.originalState internally if the navigation instance doesn't have the entry state in memory.

If history.originalState is undefined/null, history.state would be used, but now its also wrapped in a try/catch too... I expect it should never actually be called in this context.

I haven't yet looked into the prevent default issue, but I reckon this here will clear up at least some of the issues.

[fabiancook]

Looking at the click & submit callbacks, because its based on the "original event" being passed through, I added in a small test to make sure that preventDefault was being called on either intercept or preventDefault, both tests pass, I do maybe think the state of history was a bit off when the other error popped up. But with these changes it might all work together now.

https://github.com/virtualstate/navigation/blob/6602390923628d7c78d08dca22a3dddbf91b8736/src/tests/original-event.ts#L7-L27

Looking at the code here:

https://github.com/virtualstate/navigation/blob/6602390923628d7c78d08dca22a3dddbf91b8736/src/get-polyfill.ts#L279-L285

These two should line up

Btw, awesome to see formData, userInitiated, and downloadRequest being populated along with everything else! originalEvent makes perfect sense to pop on the navigationEvent too for anyone to really dig in if we missed something.

[fabiancook]

I had tried to treat history.state only as an object, but it appears it could be more than that, which is why there was originally a shift to history.state.state, but, I think we should instead use the meta object to store it, to ensure there is never a mix up of what it is. So it would be history.state[NavigationKey].state instead, but then, we should have really encoded all entry states into that history object too just to be complete, and to ignore the possibility of sessionStorage not being available...

But then whats the limitations of history.state itself, I bet it won't like storing a bunch of mixed states in there, where navigation was more made for this in mind.

---

Addressed in 3962879

[fabiancook]

I was able to make some more smaller changes, and then I added some tests specifically around anchors

The tests work across both chrome, webkit, and firefox, polyfill or not

https://github.com/virtualstate/navigation/blob/eea43f8f5fc814f0a51a23cb05f2fc6cce204761/src/tests/navigation.scope.ts#L149

https://github.com/virtualstate/navigation/blob/eea43f8f5fc814f0a51a23cb05f2fc6cce204761/src/tests/navigation.scope.ts#L211

Is polyfill? false
Scope navigation passed assertions
Anchor event seen: navigate undefined https://example.com/random/0.19146724846749352
Form event seen: navigate undefined FormData https://example.com/action/0.016781044633064512

I added a couple of logs in the polyfill just to see if we were catching the events as expected

Is polyfill? true
Scope navigation passed assertions
<-- clickCallback -->
<-- currententrychange event listener -->
Anchor event seen: navigate click https://example.com/random/0.23369991563822456
<-- submitCallback -->
<-- currententrychange event listener -->
Form event seen: navigate submit JSHandle@object https://example.com/action/0.9063574480946924

eea43f8

[fabiancook]

I added in some tests that faked the window and document just enough to be able to run through the polyfill code outside of the browser, this lets us also check coverage easier around what we have tested.

https://github.com/virtualstate/navigation/blob/feature/polyfill/src/tests/navigation.scope.faker.ts

[fabiancook]

I put together an example html document that shows the polyfill being used rolled up.

https://github.com/virtualstate/navigation/blob/9e92e486424dd51f75360e81b2e71129116e4472/example/index.html#L9

https://github.com/virtualstate/navigation/blob/9e92e486424dd51f75360e81b2e71129116e4472/example/index.html#L11-L21

I'm not 100% happy with the way window.onload is needed to be done for non-esm polyfills, if you were using an esm codebase, maybe with or without a bundler, use import "@virtualstate/navigation/polyfill" instead. I've done some changes to make sure it works with more bundlers (there appeared to be an issue on CodeSandbox where usage of const module = would be a problem)

I've confirmed in safari the history navigation works great. I can see the state making its way to session storage, and then back into the navigation once reloaded, retaining the history entries & the state related to them.

Note that not all browsers (safari) have crypto.randomUUID, this can be either polyfilled too, or a worst case id is generated in place.

As is, the polyfill appears to work "out of the box" with no issues. If everything lines up, and theres no errors thrown, all the events are caught, with anchors and forms directing to navigation instead of the window now.

Only if event.intercept or event.preventDefault are called will the default behaviour change.

If you just wanted to watch the navigation changes, hopefully the current implementation of this navigation covers all the needed steps.

[fabiancook]

I'm closing this as is, as the polyfill is usable as is, but any new issues can be created separately.

Thanks for this one, it was a huge jump forward for this project!