Add JSDoc to document plugin configs etc

[colinrotherham]

Additional documentation to help reduce bugs when working with config and plugin management code

[36degrees]

I can't see any logic to support nunjucksPath being a string rather than an array of strings – I think we'd end up iterating over each character in the string on line 48?

It looks like this might also be optional?

Should it be…

Suggested change

	* @param {string | string[]} nunjucksPaths
	* @param {string[]} [nunjucksPaths]

[colinrotherham]

Ah sorry, the cypress tests use strings and a few places in the wild too:

UKHomeOffice/home_office_design_kit/blob/master/govuk-prototype-kit.config.json
that-firefly/testing-personal-prototypes/blob/testing-eg/govuk-prototype-kit.config.json

Possibly because we documented below (bit out of date) that strings were allowed?

govuk-prototype-kit/lib/plugins/plugins.js

Lines 27 to 30 in bb9528b

	* "nunjucksPaths": string | string[],
	* "nunjucksFilters": string | string[],
	* "nunjucksFunctions": string | string[],
	* "pluginDependencies": [{"packageName": string, "minVersion": string, "maxVersion": string}],

[colinrotherham]

Definitely give #2354 a try to see lots more red squiggles

Similar to 77b7943 you'll see it's still possible to access configEntry.path and configEntry.importFrom even though configEntry might be a string etc

Or sometimes we just check the key name but not the type:

govuk-prototype-kit/lib/plugins/plugin-validator.js

Lines 188 to 190 in bb9528b

	} else if (key === 'nunjucksMacros') {
	checkNunjucksMacroExists(executionPath, configEntry.importFrom, pluginConfig.nunjucksPaths)
	} else if (typeof configEntry === 'string' && configEntry[0] === '/') {

[36degrees]

Looks like possibly the kit supports nunjucksPaths being a string but this validator doesn't allow for it?

I mean, technically you can pass a string, but it'll try and iterate over it character by character and result in errors – what are we trying to document in the JSDoc? Technical compatibility or how it's designed to be used?

[colinrotherham]

Looks like possibly the kit supports nunjucksPaths being a string but this validator doesn't allow for it?

Yeah that's it exactly

Just tried it with GOV.UK and everything works. The code was simplified recently but it's flat() that does the magic:

nunjucksViews.push(...[config.nunjucksPaths].flat()
  .map(nunjucksPath => path.join(baseDir, nunjucksPath))

[colinrotherham]

what are we trying to document in the JSDoc? Technical compatibility or how it's designed to be used?

I added JSDoc to document how the kit is currently used

That way (in theory) the compiler can error or show red squiggles on all the code that doesn't support the current usage. Except for looping strings, which is valid 🤦‍♂️

[colinrotherham]

I've pushed up a tweak to handle undefined and strings at the same time in 5e534e7

Ideally we should leave broken things broken and triage them?