-
Notifications
You must be signed in to change notification settings - Fork 334
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
Add example demonstrating the SupportError
in the browser
#4075
Conversation
packages/govuk-frontend-review/src/views/examples/js-support-error/index.njk
Outdated
Show resolved
Hide resolved
packages/govuk-frontend-review/src/views/examples/js-support-error/index.njk
Outdated
Show resolved
Hide resolved
packages/govuk-frontend-review/src/views/examples/js-support-error/index.njk
Outdated
Show resolved
Hide resolved
packages/govuk-frontend-review/src/views/examples/js-support-error/index.njk
Outdated
Show resolved
Hide resolved
packages/govuk-frontend-review/src/views/examples/js-support-error/index.njk
Outdated
Show resolved
Hide resolved
5cd2f44
to
7e90e2c
Compare
Uh oh! @romaricpascal, the image you shared is missing helpful alt text. Check your pull request body. Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image. Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs. |
1 similar comment
Uh oh! @romaricpascal, the image you shared is missing helpful alt text. Check your pull request body. Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image. Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs. |
packages/govuk-frontend-review/src/views/examples/javascript-errors/index.njk
Outdated
Show resolved
Hide resolved
packages/govuk-frontend-review/src/views/examples/javascript-errors/index.njk
Outdated
Show resolved
Hide resolved
7e90e2c
to
7f4defd
Compare
📋 StatsFile sizes
Modules
View stats and visualisations on the review app Action run for febbc72 |
Looks like I need to rebase onto main |
7f4defd
to
121d12f
Compare
packages/govuk-frontend-review/src/views/examples/javascript-errors/index.njk
Outdated
Show resolved
Hide resolved
packages/govuk-frontend-review/src/views/examples/javascript-errors/index.njk
Show resolved
Hide resolved
121d12f
to
06dd924
Compare
packages/govuk-frontend-review/src/views/examples/javascript-errors/index.njk
Outdated
Show resolved
Hide resolved
06dd924
to
8db51e0
Compare
8db51e0
to
febbc72
Compare
// Instantiate the component directly so errors are thrown | ||
new CharacterCount($element) | ||
// Use `finally` to ensure we tidy up regardless of the error | ||
} finally { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have try/finally support in our supported browsers?
Or even try-without-catch for future browser testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not aware of any support issue with try
blocks when running synchronous code. It's a feature that's been there since pretty much the start. Using it without catch
was there as early as ES5 in the specs as well (maybe even earlier, but I did not find specs for ES3 quickly)
It's a different matter for asynchronous code, though, as Promise.finally
landed more recently, so that's where we'll need to be a bit more cautious. Regarding catch
, the more recent development is the ability to omit grabbing the (error)
, for when you don't want to do anything with it (the "Optional catch binding" in the MDN support table), is that what you refer to by "try-without-catch"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…is that what you refer to by "try-without-catch"?
Yeah that's the one https://caniuse.com/mdn-javascript_statements_try_catch_optional_catch_binding
Got a bit of a gap from <script type="module">
to try...catch
: Optional catch binding
Best adding an empty catch and running the tidy up below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lack of a catch
clause has been well supported since ES5, so we're good here. The optional catch binding is about omitting the "argument" after catch
. Here's a screenshot of the page in Chrome 61 (that doesn't support Optional catch binding):
- the error is logged in the console (showing the component was instantiated with the
govuk-frontend-supported
class removed) - the
govuk-frontend-supported
class is on the<body>
(after having been added back in thefinally
block)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's good enough for me. Thanks for testing it @romaricpascal
Little follow up on #4030 to add an example for JavaScript errors to the review app. As we add more errors, we may reshape this example to show multiple kinds of errors or add extra examples for the other kinds of errors.
The example uses a CharacterCount component to trigger the errors as the component should be able to emit all the types of errors we raise, especially the one for invalid configuration that it is the only one to throw.