Skip to content

Commit

Permalink
Implement type equals module
Browse files Browse the repository at this point in the history
  • Loading branch information
BenSurgisonGDS committed Apr 5, 2023
1 parent beedf23 commit 88d0fd6
Show file tree
Hide file tree
Showing 10 changed files with 112 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,10 @@ describe('Single Plugin Test', async () => {
cy.get('.plugin-foo').should('have.css', 'background-color', BLUE)
cy.get('.plugin-foo').should('have.css', 'border-color', WHITE)
})

it('Loads plugin-foo module correctly', () => {
waitForApplication()
cy.visit('/plugin-foo')
cy.get('#foo-module').contains('The foo result is: 3')
})
})
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
{
"assets": [
"/scripts"
],
"nunjucksFilters": [
"/filters.js"
],
Expand All @@ -10,7 +13,11 @@
"/macros"
],
"scripts": [
"/scripts/foo.js"
"/scripts/foo.js",
{
"src": "/scripts/foo-module.js",
"type": "module"
}
],
"sass": [
"/sass/foo.scss"
Expand Down
10 changes: 10 additions & 0 deletions cypress/fixtures/plugins/plugin-foo/scripts/foo-module.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import fooSubmodule from './foo-submodule.js'

const fooParagraph = document.getElementById('foo-module')

if (fooParagraph) {
fooParagraph.hidden = false
setTimeout(() => {
fooParagraph.innerHTML = 'The foo result is: ' + fooSubmodule(1, 2)
}, 500)
}
3 changes: 3 additions & 0 deletions cypress/fixtures/plugins/plugin-foo/scripts/foo-submodule.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function fooSubmodule (x, y) {
return x + y
}
1 change: 1 addition & 0 deletions cypress/fixtures/plugins/plugin-foo/views/foo.njk
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
<h1 class="test-foo">Plugin Foo</h1>
<p hidden id='foo-module'>calculating</p>
8 changes: 6 additions & 2 deletions lib/nunjucks/govuk-prototype-kit/includes/scripts.njk
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
{% for scriptUrl in pluginConfig.scripts %}
<script src="{{scriptUrl}}"></script>
{% for scriptConfig in pluginConfig.scripts %}
{% if scriptConfig.type|length %}
<script type="{{scriptConfig.type}}" src="{{scriptConfig.src}}"></script>
{% else %}
<script src="{{scriptConfig.src}}"></script>
{% endif %}
{% endfor %}
4 changes: 2 additions & 2 deletions lib/plugins/plugins-routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ const plugins = require('./plugins')
function setupPathsFor (item) {
plugins.getPublicUrlAndFileSystemPaths(item)
.forEach(paths => {
requests.serveDirectory(paths.publicUrl, paths.fileSystemPath)
requests.serveDirectory(paths.publicUrl.src, paths.fileSystemPath)
// Keep /extension-assets path for backwards compatibility
// TODO: Remove in v14
requests.serveDirectory(
paths.publicUrl.replace('plugin-assets', 'extension-assets'), paths.fileSystemPath)
paths.publicUrl.src.replace('plugin-assets', 'extension-assets'), paths.fileSystemPath)
})
}

Expand Down
43 changes: 25 additions & 18 deletions lib/plugins/plugins.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,15 @@ function getPluginConfig (packageName) {

/**
* Handle errors to do with plugin paths
* @param {{ packageName: string, item: string }} subject - For example { packageName: 'govuk-frontend', item: '/all.js' }
* @throws if path in item is badly formatted
* @param {{ packageName: string, pluginPath: string }} - For example { packageName: 'govuk-frontend', pluginPath: '/all.js' }
* @throws if pluginPath is badly formatted
*/
function throwIfBadFilepath (subject) {
if (('' + subject.item).indexOf('\\') > -1) {
throw new Error(`Can't use backslashes in plugin paths - "${subject.packageName}" used "${subject.item}".`)
function throwIfBadFilepath ({ packageName, pluginPath }) {
if (('' + pluginPath).indexOf('\\') > -1) {
throw new Error(`Can't use backslashes in plugin paths - "${packageName}" used "${pluginPath}".`)
}
if (!('' + subject.item).startsWith('/')) {
throw new Error(`All plugin paths must start with a forward slash - "${subject.packageName}" used "${subject.item}".`)
if (!('' + pluginPath).startsWith('/')) {
throw new Error(`All plugin paths must start with a forward slash - "${packageName}" used "${pluginPath}".`)
}
}

Expand Down Expand Up @@ -178,18 +178,25 @@ function setPluginsByType () {

setPluginsByType()

const getPublicUrl = config => {
return ['', 'plugin-assets', config.packageName]
.concat(config.item.split('/').filter(filterOutParentAndEmpty))
const getPublicUrl = ({ packageName, item }) => {
// item will either be the plugin path or will be an object containing the plugin path within the src property
const { src, type } = typeof item === 'object' ? item : {}
const pluginPath = src || item
const urlConfig = type ? { type } : {}
urlConfig.src = ['', 'plugin-assets', packageName]
.concat(pluginPath.split('/').filter(filterOutParentAndEmpty))
.map(encodeURIComponent)
.join('/')
return urlConfig
}

function getFileSystemPath (config) {
throwIfBadFilepath(config)
function getFileSystemPath ({ packageName, item }) {
// item will either be the plugin path or will be an object containing the plugin path within the src property
const pluginPath = item.src || item
throwIfBadFilepath({ packageName, pluginPath })
return getPathFromProjectRoot('node_modules',
config.packageName,
config.item.split('/').filter(filterOutParentAndEmpty).join(path.sep))
packageName,
pluginPath.split('/').filter(filterOutParentAndEmpty).join(path.sep))
}

function getPublicUrlAndFileSystemPath (config) {
Expand Down Expand Up @@ -308,17 +315,17 @@ const getByType = type => getList(type)

/**
* Gets public urls for all plugins of type
* @param {string} type - (scripts, stylesheets, nunjucks etc)
* @param {string} listType - (scripts, stylesheets, nunjucks etc)
* @return {string[]} A list of urls
*/
const getPublicUrls = type => getList(type).map(getPublicUrl)
const getPublicUrls = listType => getList(listType).map(getPublicUrl)

/**
* Gets filesystem paths for all plugins of type
* @param {string} type - (scripts, stylesheets, nunjucks etc)
* @param {string} listType - (scripts, stylesheets, nunjucks etc)
* @return {string[]} An array of filesystem paths
*/
const getFileSystemPaths = type => getList(type).map(getFileSystemPath)
const getFileSystemPaths = listType => getList(listType).map(getFileSystemPath)

/**
* Gets public urls and filesystem paths for all plugins of type
Expand Down
90 changes: 45 additions & 45 deletions lib/plugins/plugins.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,11 @@ describe('plugins', () => {
assets: ['/ghi', '/jkl']
})
expect(plugins.getPublicUrls('assets')).toEqual([
'/plugin-assets/govuk-frontend/govuk/assets',
'/plugin-assets/another-frontend/abc',
'/plugin-assets/another-frontend/def',
'/plugin-assets/hmrc-frontend/ghi',
'/plugin-assets/hmrc-frontend/jkl'
{ src: '/plugin-assets/govuk-frontend/govuk/assets' },
{ src: '/plugin-assets/another-frontend/abc' },
{ src: '/plugin-assets/another-frontend/def' },
{ src: '/plugin-assets/hmrc-frontend/ghi' },
{ src: '/plugin-assets/hmrc-frontend/jkl' }
])
})
it('should follow strict alphabetical order when no base plugins used', () => {
Expand All @@ -148,11 +148,11 @@ describe('plugins', () => {
assets: ['/ghi', '/jkl']
})
expect(plugins.getPublicUrls('assets')).toEqual([
'/plugin-assets/another-frontend/abc',
'/plugin-assets/another-frontend/def',
'/plugin-assets/govuk-frontend/govuk/assets',
'/plugin-assets/hmrc-frontend/ghi',
'/plugin-assets/hmrc-frontend/jkl'
{ src: '/plugin-assets/another-frontend/abc' },
{ src: '/plugin-assets/another-frontend/def' },
{ src: '/plugin-assets/govuk-frontend/govuk/assets' },
{ src: '/plugin-assets/hmrc-frontend/ghi' },
{ src: '/plugin-assets/hmrc-frontend/jkl' }
])
})
it('should put specified basePlugins at the top', () => {
Expand All @@ -164,18 +164,18 @@ describe('plugins', () => {
assets: ['/ghi', '/jkl']
})
expect(plugins.getPublicUrls('assets')).toEqual([
'/plugin-assets/hmrc-frontend/ghi',
'/plugin-assets/hmrc-frontend/jkl',
'/plugin-assets/govuk-frontend/govuk/assets',
'/plugin-assets/another-frontend/abc',
'/plugin-assets/another-frontend/def'
{ src: '/plugin-assets/hmrc-frontend/ghi' },
{ src: '/plugin-assets/hmrc-frontend/jkl' },
{ src: '/plugin-assets/govuk-frontend/govuk/assets' },
{ src: '/plugin-assets/another-frontend/abc' },
{ src: '/plugin-assets/another-frontend/def' }
])
})
it('should url encode each part', () => {
mockPluginConfig('mine', { assets: ['/abc:def'] })
mockUninstallPlugin('govuk-frontend')

expect(plugins.getPublicUrls('assets')).toEqual(['/plugin-assets/mine/abc%3Adef'])
expect(plugins.getPublicUrls('assets')).toEqual([{ src: '/plugin-assets/mine/abc%3Adef' }])
})
})

Expand All @@ -190,23 +190,23 @@ describe('plugins', () => {
})
expect(plugins.getPublicUrlAndFileSystemPaths('assets')).toEqual([
{
publicUrl: '/plugin-assets/govuk-frontend/govuk/assets',
publicUrl: { src: '/plugin-assets/govuk-frontend/govuk/assets' },
fileSystemPath: path.join(rootPath, 'node_modules', 'govuk-frontend', 'govuk', 'assets')
},
{
publicUrl: '/plugin-assets/another-frontend/abc',
publicUrl: { src: '/plugin-assets/another-frontend/abc' },
fileSystemPath: path.join(rootPath, 'node_modules', 'another-frontend', 'abc')
},
{
publicUrl: '/plugin-assets/another-frontend/def',
publicUrl: { src: '/plugin-assets/another-frontend/def' },
fileSystemPath: path.join(rootPath, 'node_modules', 'another-frontend', 'def')
},
{
publicUrl: '/plugin-assets/hmrc-frontend/ghi',
publicUrl: { src: '/plugin-assets/hmrc-frontend/ghi' },
fileSystemPath: path.join(rootPath, 'node_modules', 'hmrc-frontend', 'ghi')
},
{
publicUrl: '/plugin-assets/hmrc-frontend/jkl',
publicUrl: { src: '/plugin-assets/hmrc-frontend/jkl' },
fileSystemPath: path.join(rootPath, 'node_modules', 'hmrc-frontend', 'jkl')
}
])
Expand All @@ -221,23 +221,23 @@ describe('plugins', () => {
})
expect(plugins.getPublicUrlAndFileSystemPaths('assets')).toEqual([
{
publicUrl: '/plugin-assets/another-frontend/abc',
publicUrl: { src: '/plugin-assets/another-frontend/abc' },
fileSystemPath: path.join(rootPath, 'node_modules', 'another-frontend', 'abc')
},
{
publicUrl: '/plugin-assets/another-frontend/def',
publicUrl: { src: '/plugin-assets/another-frontend/def' },
fileSystemPath: path.join(rootPath, 'node_modules', 'another-frontend', 'def')
},
{
publicUrl: '/plugin-assets/govuk-frontend/govuk/assets',
publicUrl: { src: '/plugin-assets/govuk-frontend/govuk/assets' },
fileSystemPath: path.join(rootPath, 'node_modules', 'govuk-frontend', 'govuk', 'assets')
},
{
publicUrl: '/plugin-assets/hmrc-frontend/ghi',
publicUrl: { src: '/plugin-assets/hmrc-frontend/ghi' },
fileSystemPath: path.join(rootPath, 'node_modules', 'hmrc-frontend', 'ghi')
},
{
publicUrl: '/plugin-assets/hmrc-frontend/jkl',
publicUrl: { src: '/plugin-assets/hmrc-frontend/jkl' },
fileSystemPath: path.join(rootPath, 'node_modules', 'hmrc-frontend', 'jkl')
}
])
Expand All @@ -252,23 +252,23 @@ describe('plugins', () => {
})
expect(plugins.getPublicUrlAndFileSystemPaths('assets')).toEqual([
{
publicUrl: '/plugin-assets/hmrc-frontend/ghi',
publicUrl: { src: '/plugin-assets/hmrc-frontend/ghi' },
fileSystemPath: path.join(rootPath, 'node_modules', 'hmrc-frontend', 'ghi')
},
{
publicUrl: '/plugin-assets/hmrc-frontend/jkl',
publicUrl: { src: '/plugin-assets/hmrc-frontend/jkl' },
fileSystemPath: path.join(rootPath, 'node_modules', 'hmrc-frontend', 'jkl')
},
{
publicUrl: '/plugin-assets/govuk-frontend/govuk/assets',
publicUrl: { src: '/plugin-assets/govuk-frontend/govuk/assets' },
fileSystemPath: path.join(rootPath, 'node_modules', 'govuk-frontend', 'govuk', 'assets')
},
{
publicUrl: '/plugin-assets/another-frontend/abc',
publicUrl: { src: '/plugin-assets/another-frontend/abc' },
fileSystemPath: path.join(rootPath, 'node_modules', 'another-frontend', 'abc')
},
{
publicUrl: '/plugin-assets/another-frontend/def',
publicUrl: { src: '/plugin-assets/another-frontend/def' },
fileSystemPath: path.join(rootPath, 'node_modules', 'another-frontend', 'def')
}
])
Expand All @@ -277,7 +277,7 @@ describe('plugins', () => {
mockPluginConfig('mine', { assets: ['/abc:def'] })
mockUninstallPlugin('govuk-frontend')

expect(plugins.getPublicUrls('assets')).toEqual(['/plugin-assets/mine/abc%3Adef'])
expect(plugins.getPublicUrls('assets')).toEqual([{ src: '/plugin-assets/mine/abc%3Adef' }])
})
it('should not break when asking for an plugin key which isn\'t used', () => {
expect(plugins.getPublicUrls('anotherListThatDoesntExist')).toEqual([])
Expand Down Expand Up @@ -363,12 +363,12 @@ describe('plugins', () => {
it('should not include public URLs for jQuery if it is not installed', () => {
expect(plugins.getPublicUrlAndFileSystemPaths('assets')).toEqual([{
fileSystemPath: path.resolve('node_modules', 'jquery', 'dist'),
publicUrl: '/plugin-assets/jquery/dist'
publicUrl: { src: '/plugin-assets/jquery/dist' }
}])
})
it('should not include scripts for jQuery if it is not installed', () => {
expect(plugins.getAppConfig().scripts).toEqual([
'/plugin-assets/jquery/dist/jquery.js'
{ src: '/plugin-assets/jquery/dist/jquery.js' }
])
})
})
Expand All @@ -385,7 +385,7 @@ describe('plugins', () => {

it('should return a list of public urls for the scripts', () => {
expect(plugins.getAppConfig().scripts).toEqual([
'/plugin-assets/govuk-frontend/govuk/all.js'
{ src: '/plugin-assets/govuk-frontend/govuk/all.js' }
])
})

Expand All @@ -396,16 +396,16 @@ describe('plugins', () => {
it('should include installed plugins where scripts config is a string array', () => {
mockPluginConfig('my-plugin', { scripts: ['/abc/def/ghi.js'] })
expect(plugins.getAppConfig().scripts).toEqual([
'/plugin-assets/govuk-frontend/govuk/all.js',
'/plugin-assets/my-plugin/abc/def/ghi.js'
{ src: '/plugin-assets/govuk-frontend/govuk/all.js' },
{ src: '/plugin-assets/my-plugin/abc/def/ghi.js' }
])
})

it('should include installed plugins where scripts config is a string', () => {
mockPluginConfig('my-plugin', { scripts: '/ab/cd/ef/ghi.js' })
expect(plugins.getAppConfig().scripts).toEqual([
'/plugin-assets/govuk-frontend/govuk/all.js',
'/plugin-assets/my-plugin/ab/cd/ef/ghi.js'
{ src: '/plugin-assets/govuk-frontend/govuk/all.js' },
{ src: '/plugin-assets/my-plugin/ab/cd/ef/ghi.js' }
])
})

Expand All @@ -416,21 +416,21 @@ describe('plugins', () => {
it('should include installed plugins', () => {
mockPluginConfig('my-plugin', { stylesheets: ['/abc/def/ghi.css'] })
expect(plugins.getAppConfig().stylesheets).toEqual([
'/plugin-assets/my-plugin/abc/def/ghi.css'
{ src: '/plugin-assets/my-plugin/abc/def/ghi.css' }
])
})

it('should allow core stylesheets and scripts to be passed in', () => {
mockPluginConfig('my-plugin', { stylesheets: ['/abc/def/ghi.css'], scripts: ['/jkl/mno/pqr.js'] })
expect(plugins.getAppConfig({ stylesheets: ['/a.css', '/b.css'], scripts: ['/d.js', 'e.js'] })).toEqual({
stylesheets: [
'/plugin-assets/my-plugin/abc/def/ghi.css',
{ src: '/plugin-assets/my-plugin/abc/def/ghi.css' },
'/a.css',
'/b.css'
],
scripts: [
'/plugin-assets/govuk-frontend/govuk/all.js',
'/plugin-assets/my-plugin/jkl/mno/pqr.js',
{ src: '/plugin-assets/govuk-frontend/govuk/all.js' },
{ src: '/plugin-assets/my-plugin/jkl/mno/pqr.js' },
'/d.js',
'e.js'
]
Expand All @@ -444,8 +444,8 @@ describe('plugins', () => {
mockPluginConfig('another-fixable-plugin', { stylesheets: '/abc.css' })

expect(plugins.getAppConfig().stylesheets).toEqual([
'/plugin-assets/another-fixable-plugin/abc.css',
'/plugin-assets/my-fixable-plugin/abc.css'
{ src: '/plugin-assets/another-fixable-plugin/abc.css' },
{ src: '/plugin-assets/my-fixable-plugin/abc.css' }
])
})
it('should throw if paths use backslashes', () => {
Expand Down
Loading

0 comments on commit 88d0fd6

Please sign in to comment.