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

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

Open
peterbe opened this issue Sep 14, 2020 · 7 comments
Open

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

peterbe opened this issue Sep 14, 2020 · 7 comments

Comments

@peterbe
Copy link

peterbe commented Sep 14, 2020

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
Copy link
Author

peterbe commented Sep 14, 2020

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
Copy link
Member

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
Copy link
Member

developit commented Sep 14, 2020

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
Copy link
Author

peterbe commented Sep 14, 2020

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

@peterbe
Copy link
Author

peterbe commented Sep 14, 2020

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
Copy link
Member

developit commented Sep 14, 2020

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

@peterbe
Copy link
Author

peterbe commented Sep 15, 2020

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 preactjs/preact-cli#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants