alphagov / govuk-frontend

GOV.UK Frontend contains the code you need to start building a user interface for government platforms and services.
https://frontend.design-system.service.gov.uk/
MIT License
1.16k stars 320 forks source link

Investigate throwing errors or warnings when components do not get required inputs #562

Closed NickColley closed 6 years ago

NickColley commented 6 years ago

Ideally our components would loudly complain if they're supplied with bad inputs.

This currently isn't trivial with our current Nunjucks setup.

Related comment: https://github.com/alphagov/govuk-frontend/issues/558#issuecomment-369239872

See this skipped test: https://github.com/alphagov/govuk-frontend/blob/master/src/back-link/template.test.js#L17

joelanman commented 6 years ago

tricky, as the easiest options (putting an error on the page, or writing an error to the console) are end-user facing. End users (the public) can't do anything about invalid usage of our macros, so we really should avoid them getting errors if possible.

NickColley commented 6 years ago

This has worked really well for GOV.UK Publishing Components.

It's much better to have loud warnings that can be picked up before deploying as part of tests and monitoring than to have silent failures that are picked up by end-users.

Example test: https://github.com/alphagov/govuk_publishing_components/blob/master/spec/components/radio_test_spec.rb#L12

joelanman commented 6 years ago

good point. I can't see what the code in that link is doing? Does it write a big red message to the page or something?

NickColley commented 6 years ago

@joelanman that test is making sure when a component doesn't get the right input it fails.

36degrees commented 6 years ago

In Ruby land you can raise an exception, but Nunjucks has no such equivalent so (as far as I know).

NickColley commented 6 years ago

At least in the Ruby ERB templates, they will raise an exception if something is not passed through by default. We have an empty object passed through to our templates that may mask a default behaviour if Nunjucks has it, would be worth investigating that.

36degrees commented 6 years ago

Closing this as this isn't something Nunjucks supports which makes this very difficult to do. It could be a good thing to look into in the next firebreak.