createCacheEntry() fails if page has data-taxi-view attribute but no custom Renderer

[vallafederico]

Describe the bug createCacheEntry() fails if page has data-taxi-view attribute specificed and no custom Renderer.

Not totally sure this is a bug or intended behaviour tbh, I felt as if it was a bug as it was unexpected and took me a minute to actually figure out.

To Reproduce Minimal StackBlitz Example. You can switch from main.js to fixed.jsin home page script to simulate what the behaviour I'd consider normal should be. Two pages, data-taxi-view="home" & data-taxi-view="about".

main.js

import { Core, Renderer, Transition } from '@unseenco/taxi';

const taxi = new Core({
  renderers: {
    default: Renderer,
  },
  transitions: {
    default: Transition,
  },
});

renderers and transition can even be avoided but put them there just to make sure that wouldn't somehow fix it.

Info Error in the console

@unseenco_taxi.js?v=bde4e522:835 Uncaught TypeError: Renderer2 is not a constructor
    at Core.createCacheEntry (@unseenco_taxi.js?v=bde4e522:835:17)
    at new Core (@unseenco_taxi.js?v=bde4e522:650:52)
    at main.js:3:14

Looking into it Renderer is actually undefined.

Maybe Fix?

  createCacheEntry(page) {
    const content = page.querySelector('[data-taxi-view]');

    let Renderer = content.dataset.taxiView.length
      ? this.renderers[content.dataset.taxiView]
      : this.defaultRenderer;

    /* 
      content.dataset.taxiView.length is actually true
      (as the view exist as an attribute in html)
      but this.renderers[content.dataset.taxiView] is undefined
      (as there's no renderer provided in js)
    */

    if (Renderer === undefined) Renderer = this.defaultRenderer;
    /*
      fix can be as simple as this, just providing an escape
      if Renderer is undefined 
    */

    return {
      page,
      content,
      skipCache: content.hasAttribute('data-taxi-nocache'),
      scripts: this.reloadJsFilter
        ? Array.from(page.querySelectorAll('script')).filter(
            this.reloadJsFilter
          )
        : [],
      title: page.title,
      renderer: new Renderer({
        wrapper: this.wrapper,
        title: page.title,
        content,
        page,
      }),
    };
  }

Am I missing something? Is it the actual expected behaviour? Still, thanks for the amazing work in any case and sorry to bother ❤️

[jakewhiteley]

This is the expected behaviour, but just not a useful error message.

So your pages have home and about declared as the renderers you want to use, they have just not been registered with Taxi yet.

const taxi = new Core({
  renderers: {
    default: Renderer,
    home: HomeRenderer,
    about: AboutRenderer,
  }
});

While Taxi could fall back to the default renderer in these situations, I would say that would cause bugs which would be harder to debug: If you had created a HomeRenderer and an AboutRenderer and forgotten to register them, the page would still load but none of your specific renderer code would be executed, leading you down a long path to try and work out why.

I have added a console.warn and updated the npm package 👍

https://github.com/craftedbygc/taxi/blob/7e793185ac1206cc69895835528907a552c42ac8/src/Core.js#L456

So now if you run into this issue, at least working out the cause will be considerably easier!

[vallafederico]

I guess it makes sense!

Thanks @jakewhiteley amazing you're doing here.