Merge pull request #2088 from alphagov/implement-script-type-equals-m…

…odule

Plugin authors can now use scripts with type module

cypress/e2e/plugins/0-mock-plugin-tests/single-plugin-test.cypress.js

@@ -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')
+  })
 })

cypress/fixtures/plugins/plugin-foo/govuk-prototype-kit.config.json

@@ -1,4 +1,7 @@
 {
+  "assets": [
+    "/scripts"
+  ],
   "nunjucksFilters": [
     "/filters.js"
   ],
@@ -10,7 +13,11 @@
     "/macros"
   ],
   "scripts": [
-    "/scripts/foo.js"
+    "/scripts/foo.js",
+    {
+      "path": "/scripts/foo-module.js",
+      "type": "module"
+    }
   ],
   "sass": [
     "/sass/foo.scss"

cypress/fixtures/plugins/plugin-foo/scripts/foo-module.js

@@ -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)
+}

cypress/fixtures/plugins/plugin-foo/scripts/foo-submodule.js

@@ -0,0 +1,3 @@
+export default function fooSubmodule (x, y) {
+  return x + y
+}

cypress/fixtures/plugins/plugin-foo/views/foo.njk

@@ -1 +1,2 @@
 <h1 class="test-foo">Plugin Foo</h1>
+<p hidden id='foo-module'>calculating</p>

lib/nunjucks/govuk-prototype-kit/includes/scripts.njk

@@ -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 %}

lib/plugins/plugins.js

@@ -185,17 +185,21 @@ const getPublicUrl = config => {
     .join('/')
 }
 
-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
+  item = item.path || item
+  throwIfBadFilepath({ packageName, item })
   return getPathFromProjectRoot('node_modules',
-    config.packageName,
-    config.item.split('/').filter(filterOutParentAndEmpty).join(path.sep))
+    packageName,
+    item.split('/').filter(filterOutParentAndEmpty).join(path.sep))
 }
 
-function getPublicUrlAndFileSystemPath (config) {
+function getPublicUrlAndFileSystemPath ({ packageName, item }) {
+  // item will either be the plugin path or will be an object containing the plugin path within the src property
+  item = item.path || item
   return {
-    fileSystemPath: getFileSystemPath(config),
-    publicUrl: getPublicUrl(config)
+    fileSystemPath: getFileSystemPath({ packageName, item }),
+    publicUrl: getPublicUrl({ packageName, item })
   }
 }
 
@@ -308,17 +312,26 @@ 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(({ packageName, item }) => {
+  // item will either be the plugin path or will be an object containing the plugin path within the src property
+  if (listType === 'scripts' && typeof item === 'object') {
+    const { path, type } = item
+    const publicUrl = getPublicUrl({ packageName, item: path })
+    return { src: publicUrl, type }
+  } else {
+    return getPublicUrl({ packageName, item })
+  }
+})
 
 /**
  * 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
@@ -330,12 +343,12 @@ const getPublicUrlAndFileSystemPaths = type => getList(type).map(getPublicUrlAnd
 /**
  * This is used in the views to output links and scripts for each file
  * @param {{scripts: string[], stylesheets: string[]}} additionalConfig
- * @return {{scripts: string[], stylesheets: string[]}} Returns an object containing two keys(scripts & stylesheets),
+ * @return {{scripts: {src: string, type: string}[], stylesheets: string[]}} Returns an object containing two keys(scripts & stylesheets),
  *   each item contains an array of full paths to specific files.
  */
 function getAppConfig (additionalConfig) {
   return {
-    scripts: self.getPublicUrls('scripts').concat((additionalConfig || {}).scripts || []),
+    scripts: self.getPublicUrls('scripts').concat((additionalConfig || {}).scripts || []).map((item) => typeof item === 'string' ? { src: item } : item),
     stylesheets: self.getPublicUrls('stylesheets').concat((additionalConfig || {}).stylesheets || [])
   }
 }

lib/plugins/plugins.test.js

@@ -368,7 +368,7 @@ describe('plugins', () => {
       })
       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' }
         ])
       })
     })
@@ -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' }
       ])
     })
 
@@ -396,16 +396,24 @@ 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' }
+      ])
+    })
+
+    it('should include installed plugins where scripts config is an object including type', () => {
+      mockPluginConfig('my-plugin', { scripts: { path: '/ab/cd/ef/ghi.js', type: 'module' } })
+      expect(plugins.getAppConfig().scripts).toEqual([
+        { src: '/plugin-assets/govuk-frontend/govuk/all.js' },
+        { src: '/plugin-assets/my-plugin/ab/cd/ef/ghi.js', type: 'module' }
       ])
     })
 
@@ -429,10 +437,10 @@ describe('plugins', () => {
           '/b.css'
         ],
         scripts: [
-          '/plugin-assets/govuk-frontend/govuk/all.js',
-          '/plugin-assets/my-plugin/jkl/mno/pqr.js',
-          '/d.js',
-          'e.js'
+          { src: '/plugin-assets/govuk-frontend/govuk/all.js' },
+          { src: '/plugin-assets/my-plugin/jkl/mno/pqr.js' },
+          { src: '/d.js' },
+          { src: 'e.js' }
         ]
       })
     })