Skip to content

Conversation

@BenSurgisonGDS
Copy link
Contributor

@BenSurgisonGDS BenSurgisonGDS commented Aug 30, 2023

Remove hard coded references for the locations of the govuk-frontend scripts and assets.

@BenSurgisonGDS BenSurgisonGDS force-pushed the get-scripts-and-assets-from-config branch 2 times, most recently from 4d31fab to d8276fb Compare August 30, 2023 12:47
@BenSurgisonGDS BenSurgisonGDS marked this pull request as ready for review August 30, 2023 13:02
@BenSurgisonGDS BenSurgisonGDS changed the base branch from main to using-internal-govuk-frontend-more-widely August 30, 2023 13:16
Base automatically changed from using-internal-govuk-frontend-more-widely to main August 31, 2023 08:42
@BenSurgisonGDS BenSurgisonGDS force-pushed the get-scripts-and-assets-from-config branch from 5895226 to 03bd181 Compare September 11, 2023 10:17

if ('govuk-frontend' in dependencies) {
const frontEndConfigFile = path.join(config.env.projectFolder, 'node_modules', 'govuk-frontend', 'govuk-prototype-kit.config.json')
config.env.frontendAssetsFolder = fse.readJsonSync(frontEndConfigFile).assets.find(asset => !asset.split(path.sep).pop().includes('.'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This closely targets the current implementation inside govuk-frontend - if they add another assets item to their config which doesn't have any dots then this may identify the wrong one.

<script src="/manage-prototype/dependencies/govuk-frontend/govuk/all.js"></script>
<script src="/manage-prototype/dependencies/govuk-frontend/govuk-prototype-kit/init.js"></script>
<script src="/plugin-assets/govuk-prototype-kit/lib/assets/javascripts/kit.js"></script>
{% for scriptConfig in managePlugins.scripts %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be a duplicate of govuk-prototype-kit/includes/scripts.njk, and both are taking their input from the plugin config. We should reuse the existing include rather than copying it. This can be done by calling the field in the model pluginConfig rather than managePlugins.

@BenSurgisonGDS BenSurgisonGDS force-pushed the get-scripts-and-assets-from-config branch 2 times, most recently from eb696aa to 532b418 Compare September 28, 2023 13:41
@BenSurgisonGDS BenSurgisonGDS force-pushed the get-scripts-and-assets-from-config branch from 532b418 to 9c11033 Compare September 28, 2023 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants