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

Array.prototype.includes sample does not trap errors so output is empty #218

Closed
stuartlangridge opened this issue Oct 6, 2015 · 2 comments
Assignees
Labels

Comments

@stuartlangridge
Copy link

stuartlangridge commented Oct 6, 2015

https://googlechrome.github.io/samples/array-includes-es7/index.html has the "Live Output" box empty in a browser not supporting Array.prototype.includes, because the first example (if (basketOfApples.includes('🌰'))) fails (because of an Uncaught TypeError: basketOfApples.includes is not a function) and so the remainder of the code is not executed. Perhaps it would be useful if errors were trapped and some sort of "failure" message added to the Live Output section, so that one can see that one's current browser does not support this sample (and one can tell the difference between "my browser tried to do this and failed because it's not new enough" and "the code is broken or just doesn't work").
(This is likely not specific to this particular sample and is probably a more general class of error in the samples as a whole.)

@addyosmani addyosmani added the bug label Oct 6, 2015
@addyosmani
Copy link
Member

addyosmani commented Oct 6, 2015

There are a few different ways we could tackle this:

  • Explicitly feature detect support (where possible) and gracefully fail on a per sample basis
  • Come up with a general catch-all that informs the end user an error occurred which may be related to lack of browser support

In the short term, I like the idea of a catch-all as it's better than the current behaviour of not displaying any really useful output to the user. Interested in @jeffposnick's thoughts here.

@jeffposnick
Copy link
Contributor

jeffposnick commented Oct 6, 2015

Thanks for pointing this out. It's probably safe to add the following to our sample template:

window.addEventListener('error', function(error) {
  if (ChromeSamples && ChromeSamples.log) {
    ChromeSamples.log(error.message, '\n  (Your browser does not currently support this feature.)');
    error.preventDefault();
  }
});

Which would look like

image

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

No branches or pull requests

3 participants