For discussion: Import all components in template

[frankieroberto]

This updates the main template.njk file, which is exported in the nhsuk-frontend package, so that it imports all of the components available, rather than just the small number used in that template file.

The main advantage of this would be that users of NHS frontend no longer have to remember to import the components, either within individual view files, or within their own template.

For the NHS prototype kit, that currently imports all components in prototype-kit-template.njk. However, current if a minor version of an NHS frontend release introduces a new component, then we have to remember to update the template in the prototype kit to add the new import, and then release a new version of the kit.

If NHS Frontend includes the imports itself, then users of the prototype kit would be able to upgrade NHS frontend without needing to wait for an update of the prototype kit.

Importing all components by default could also allow us to drop the individual component imports from the code examples on the service manual in future:

This might help those prototyping by avoiding their prototype code being littered with unneeded import lines.

However we might not want to drop the import lines from the examples straight away, as it would take people a while to upgrade, and having unnecessary import lines is probably better than not having them and the examples breaking for users who haven’t yet upgraded?

[colinrotherham]

I love this idea and it's definitely a good responsibility for NHS.UK frontend

I do wonder if template.njk should stay non-conflicting?

Avoiding conflicts

We'd probably want to preserve the last inherited approach we have to avoid conflicts:

• Component prefix nhsuk used to differentiate components
• Directory prefix nhsuk/ used to differentiate paths

{% from "app/components/button/macro.njk" import button %}
{% from "govuk/components/button/macro.njk" import govukButton %}
{% from "nhsuk/components/button/macro.njk" import button as nhsukButton %}

[frankieroberto]

@colinrotherham I’m not yet convinced we need to add an nhsuk prefix to all our components - that would be quite a big breaking changing!

There’s probably an argument that we should have followed govuk-frontend in prefixing our components, but we are where we are and I don’t think it’s caused a ton of problems?

If you end up adding your own component which has the same name as an NHS one, you have 2 options:

• import your component from your template using the same name as the NHS one, which will override it as it’s imported after
• import your component with a different prefix, like appButton

either might be fine, depending on your context?

[colinrotherham]

@colinrotherham I’m not yet convinced we need to add an nhsuk prefix to all our components - that would be quite a big breaking changing!

Yeah agreed, I've put a big line through it

What I mean is that this PR can force all our components to take priority, depending on load path order

i.e. Your local button() gets ignored, even if you imported the NHS one as nhsukButton() 🔥🔥🔥

Breaking or non-breaking

We can get this into v10.x by having two templates:

• template.njk without component and macro imports
• template-imports.njk with component and macro imports

The prototype kit can use template-imports.njk?

It could even have its own local template.njk that wraps template-imports.njk for compatibility?

Otherwise it's a breaking change

[edwardhorsford]

I think user imports should probably take precedence over any nhs ones.

[frankieroberto]

template-imports.njk would be fine by me, at least until the next breaking change release.

We'd then have prototype-kit-template.njk extend that instead of template.njk. It’d then be a non-breaking change for the prototype kit.

That said, I’m not sure that adding the imports directly to template.njk would be a breaking change? It’d be backwards-compatible, as there would be no change if you have already imported components separately, either in individual views or in a template?

[colinrotherham]

That said, I’m not sure that adding the imports directly to template.njk would be a breaking change? It’d be backwards-compatible, as there would be no change if you have already imported components separately, either in individual views or in a template?

Me neither, I wasn't 100% sure when I said "depending on load path order" above

We do already have a slight issue, unrelated but similar

Take fileUpload() for example

If a service team has their own local fileUpload() macro and they update to NHS.UK frontend v10.3.0, our recommend load paths mean this import may suddenly match our version first—"depending on load path order"

{% from "components/file-upload/macro.njk" import fileUpload %}

But in this PR's case, what about teams that already have fileUpload() imported globally at template level?

I'm a bit rusty but I can't remember the precedence across extended templates and/or includes

[colinrotherham]

Useful context:

• Allow importing Nunjucks macros with a namespace without mounting 'node_modules' alphagov/govuk-frontend#870
• Namespace Frontend inside a govuk directory alphagov/govuk-frontend#1451
• Add proposal to namespace GOV.UK Frontend Nunjucks and Sass using a nested folder structure alphagov/govuk-design-system-architecture#15

Which eventually led to the decision:

• Namespace GOV.UK Frontend Nunjucks and Sass using a nested folder structure

[colinrotherham]

Want to add it as template-imports.njk (non-breaking) and target v10?

[frankieroberto]

@colinrotherham I’ve checked, and you can override imported components with your own components - the last-imported component seems to take precedence.

So this for instance seems to work fine:

{% extends "prototype-kit-template.njk" %}

{# Override NHS button component with a custom one #}
{% from '../components/button/macro.njk' import button %}

[frankieroberto]

The as syntax for importing does not seem to override already-imported components, however, so this does not work:

{% extends "prototype-kit-template.njk" %}

{% This does not seem to work: #}
{% from '../components/app_button/macro.njk' import appButton as button %}

Not sure if that’s a Nunjucks bug!

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud