search

erikringsmuth / app-router

Router for Web Components
https://erikringsmuth.github.io/app-router/
MIT License
610 stars 83 forks source link

Add HTML import error callback #152

Closed web-padawan closed 7 years ago

web-padawan commented 7 years ago

As it is described in the spec, there can be thrown an error event when HTMl import was failed: 

The link element fires a simple event called load for successful loading attempt. 
For failed attempt, it fires a simple event named error.

I created this PR to catch 404 errors when performing import. My point is that app-router should itself fire an event containing URL of requested <link> resource.

@erikringsmuth @jmalonzo PTAL.

See also #101

jmalonzo commented 7 years ago

@web-padawan Thanks for this. I kinda prefer @nomego's approach here. If I merge that, can you please add the Uri and importUri fields in to the event detail?

Thanks.

[Edit: I only liked PR #101 due to the fact that it fires the error on both the route and router. But I think having the URI in the detail is helpful]

web-padawan commented 7 years ago

Sure, I'll update this PR to fit your feedback.

jmalonzo commented 7 years ago

@web-padawan Thanks. I just merged PR #101.

web-padawan commented 7 years ago

@jmalonzo that's done. I rebased my PR and added properties to event detail object.

It would be nice to run build and to tag those changes with a new version tag, WDYT?

web-padawan commented 7 years ago

UPD: routeDetail already contains reference to app-route node, as well as the path, so maybe duplicating them is obsolete?

jmalonzo commented 7 years ago

Hmm yeah you're right. I think the current routeDetail should suffice?

web-padawan commented 7 years ago

I agree. Will close the PR in favor of using that. Thanks!