preactjs / preact-router

:earth_americas: URL router for Preact.
http://npm.im/preact-router
MIT License
1.02k stars 155 forks source link

[feature request] onError handler to catch when lazy-loading fails #380

Open peterbe opened 4 years ago

peterbe commented 4 years ago

I'd love to be able to do this:

        <Router onError={handleRouteError}>
          <Route
            path="/"
            component={Home}
          />
</Router>

That way I could be informed when preact-router failed. For example, somewhere deep inside <Route/> it's going to attempt to download a .js file based on the route. Something like build/route-home/index.tsx.chunk.887b9.esm.js. That might fail because of network or because it's 404'ing because the main bundle is older than what's available on the server. Somewhere somehow I'd like for the lazy() call to be caught and bubbled up so I can control it.

If this was possible, I'd be able to re-render my app and say something like "Sorry. An error occurred trying to load this route. Try reloading your browser."

peterbe commented 4 years ago

This rather desperate discussion is relevant: https://gitter.im/webpack/webpack?at=5d762c273b1e5e5df17c91f7

The problem is that somewhere Webpack does inject the script tag into the DOM based on the Webpack manifest. That script tag has its own onerror but it's not being handled and and somewhere that ability is lost in preact-router.

developit commented 4 years ago

I mentioned it on slack, but preact-router doesn't have any knowledge of async components - that's a preact-cli thing the router can't see.

developit commented 4 years ago

I'm going to cross-post this from Slack for others - here are two options for handling preact-cli async component loading errors using preact-router.

Option 1: Simple

You can use Router's onChange event to check if a route failed to load:

function routeChanged({ current, url, previous, router }) {
  // preact-cli's async routes have a .preload() method
  const component = current && current.type;
  if (component && component.preload) {
    component.preload(mod => {
      // loading succeeded:
      if (mod) return;
      // ...otherwise, the route resolved to a failed/empty module.
      // Some options for how to handle this state:
      // Option 1 - show the previous route:
      router.routeTo(previous);
      // Option 2 - show an error page:
      router.routeTo('/error');
      // Option 3 - show an error modal:
      this.setState({ error: `Failed to load route ${url}` });
    });
  }
}

Option 2: Smart

This is nice because it's a drop-in replacement for preact-router, and it actually improves performance. It also makes it easy to add a "page loading" spinner/progress indicator. I use a design like this in most of my demos. Essentially, any time the Router is given a new URL to display, it first preloads the matched component for that route before switching to it - that saves a pointless render that would have blanked the screen, and also makes it possible to catch loading errors at the router:

import { toChildArray } from 'preact';
import Router from 'preact-router';

export class SmartRouter extends Router {
  routeTo(url) {
    this._pendingUrl = url;
    let { children, onLoading, onLoaded, onError } = this.props;
    const route = this.getMatchingChildren(toChildArray(children), url, false)[0];
    if (!route) return false; // 404
    const Route = route.type;
    if (!Route || !Route.preload || Route.loaded) {
      return super.routeTo(url);
    }
    const ctx = { url, current: route };
    if (onLoading) onLoading(ctx);
    Route.preload(mod => {
      if (this._pendingUrl != url) return;
      Route.loaded = true;
      if (!mod) ctx.error = Error(`Failed to load ${url}`);
      if (onLoaded) onLoaded(ctx);
      if (mod) super.routeTo(url);
      else if (onError) onError(ctx);
      else setTimeout(() => { throw err; });
    });
    return true;
  }
}

usage:

import { Component } from 'preact';
import Home from '../routes/home';
import Profile from '../routes/profile';
import Settings from '../routes/settings';

export default class App extends Component {
  loadStart({ url, current }) {
    document.getElementById('progress').removeAttribute('hidden');
  }
  loadEnd({ url, current, error }) {
    document.getElementById('progress').setAttribute('hidden', '');
    // note: `error` is defined here if there was an error (can use instead of onError)
  }
  onError({ url, error }) {
    alert(`Failed to load ${url}: ${error}`);
  }
  render() {
    return (
      <SmartRouter onLoading={this.loadStart} onLoaded={this.loadEnd} onError={this.onError}>
        <Home path="/" />
        <Profile path="/profile" username="me" />
        <Profile path="/profile/:username" />
        <Settings path="/settings" />
        <NotFound default />
      </SmartRouter>
    );
  }
}

const NotFound = () => <div class="error"><h1>404: Not Found</h1></div>
peterbe commented 4 years ago

@developit You never cease to impress me. I'll give this a shot and report back if the stopgap solution works.

peterbe commented 4 years ago

I haven't really grokked what it does yet but I started implementing use of that SmartRouter and noticed I had to make this change:

+import { toChildArray } from "preact";
import { Router } from "preact-router";

export class SmartRouter extends Router {
  routeTo(url) {
    this._pendingUrl = url;
-   let { children, onLoading, onLoaded, onError } = this.props;
-   children = [children].flatten();
+   const { onLoading, onLoaded, onError } = this.props;
+   const children = toChildArray(this.props.children);
    const route = this.getMatchingChildren(children, url, false)[0];
    if (!route) return false; // 404
    const Route = route.type;

Seems to work so far. Otherwise you get a TypeError on [...].flatten().

I haven't actually started testing the use of onError yet (when the chunk is gone). Thanks again.

developit commented 4 years ago

heh - yeah that's a much better solution @peterbe, ~I'll update~ I updated the code.

peterbe commented 4 years ago

Unfortunately, I couldn't make it work. First I tried to convert that snippet to TypeScript and failed. Sigh. I'm not a TypeScript expert so I couldn't figure out to get all the errors go away. :( Then I tried the simple solution and it doesn't work. I don't know why.

Here's how to reproduce the problem:

(I was going to demo the steps to a reproducible complete example, but I couldn't because of https://github.com/preactjs/preact-cli/issues/1425)

But in my app, I can easily break it by running yarn run build, start the app. Then, rm build/route-home/index.tsx.chunk.dfe09.esm.js and then reload the app in the browser.

Screen Shot 2020-09-14 at 9 56 40 PM

In a sense, it's kinda obvious that it shouldn't work if one of the script bundles (chunk) is missing but although manually deleting the file is unrealistic, what's happening in my case is that the reason you get a 404 is because of a stale service worker or a stale CDN since every yarn run build resets the ./build/ what I have on my hosting server is only the latest and greatest.