feat(critical-css): accept requiredClassNames and retries at config level

packages/sui-critical-css/README.md

@@ -27,10 +27,30 @@ In order to extract critical css and match extracted files with your page or rou
 
 You can combine both of them.
 
-Additionally there are two optional config parameters:
+Additionally there are two optional parameters:
 
-- `requiredClassNames`: A list of required css class names. If they aren't present in the generated Critical CSS, it would be discarded. By default there would be 2 retries to try to get the correct Critical CSS
-- `retries`: Number of retries if the requiredClassNames aren't present in the Critical CSS. By default it's 2.
+- `requiredClassNames`: A list of required css class names. If they aren't present in the generated Critical CSS, the extraction is retried, and an empty Critical CSS is written once the attempts run out.
+- `retries`: How many times a route is extracted while its `requiredClassNames` aren't present. By default it's 3.
+
+Both can be set on `config`, where they apply to every route, and on a single route, where they
+override the config. Setting them on `config` is the only way to validate a route declared as a
+plain string, since such a route cannot carry options of its own:
+
+```js
+const config = {
+  hostname: 'http://localhost',
+  requiredClassNames: ['.sui-AtomButton'], // checked on every route below
+  retries: 2
+}
+
+const routes = {
+  '/:lang': '/es',
+  '/:lang/catalogo-productos': {
+    url: '/es/catalogo-productos',
+    requiredClassNames: ['.ma-AdCardV2'] // only this list is checked here
+  }
+}
+```
 
 ### Using `path-to-regex`:
 

packages/sui-critical-css/src/extract-from-app.js

@@ -9,6 +9,7 @@ import {extractCSSFromUrl} from './extract-from-url.js'
 
 const TIME_BETWEEN_RETRIES = 1000
 const TIMES_TO_RETRY = 15
+const DEFAULT_RETRIES = 3
 
 const configForMobileDevice = devices.m
 
@@ -17,6 +18,20 @@ export const createUrlFrom = ({hostname, pathOptions}) => {
   return Array.isArray(path) ? path.map(current => `${hostname}${current}`) : `${hostname}${path}`
 }
 
+/**
+ * `requiredClassNames` and `retries` can be set on `config`, where they apply to every route, and
+ * on a route, where they override the config. A route given as a plain string carries neither, so
+ * without the config level there was no way to validate one at all.
+ */
+export const resolveRouteOptions = ({pathOptions, config = {}}) => {
+  const routeOptions = typeof pathOptions === 'string' || !pathOptions ? {} : pathOptions
+
+  return {
+    requiredClassNames: routeOptions.requiredClassNames ?? config.requiredClassNames,
+    retries: routeOptions.retries ?? config.retries ?? DEFAULT_RETRIES
+  }
+}
+
 const waitForHealthCheck = ({healthCheckUrl}) => {
   return new Promise(resolve => {
     async function retry(retries) {
@@ -36,7 +51,7 @@ const waitForHealthCheck = ({healthCheckUrl}) => {
   })
 }
 
-const extractCriticalCSS = async ({requiredClassNames, retries = 3, url, configForMobileDevice} = {}) => {
+const extractCriticalCSS = async ({requiredClassNames, retries = DEFAULT_RETRIES, url, configForMobileDevice} = {}) => {
   if (retries === 0) {
     console.log(`Attempt limit reached. requiredClassNames has not been found in ${url}`)
     return ''
@@ -99,7 +114,7 @@ export async function extractCSSFromApp({routes, config = {}}) {
 
     manifest[pathKey] = cssFileName
 
-    const {requiredClassNames, retries} = pathOptions
+    const {requiredClassNames, retries} = resolveRouteOptions({pathOptions, config})
     const css = await extractCriticalCSS({
       requiredClassNames,
       retries,

packages/sui-critical-css/test/server/extract-from-appSpec.js

@@ -0,0 +1,84 @@
+import {expect} from 'chai'
+
+import {createUrlFrom, resolveRouteOptions} from '../../src/extract-from-app.js'
+
+describe('@s-ui/critical-css extract-from-app', () => {
+  describe('createUrlFrom', () => {
+    it('should build the url from a route given as a plain string', () => {
+      expect(createUrlFrom({hostname: 'http://localhost', pathOptions: '/es'})).to.equal('http://localhost/es')
+    })
+
+    it('should build the url from the "url" of a route given as an object', () => {
+      expect(createUrlFrom({hostname: 'http://localhost', pathOptions: {url: '/es'}})).to.equal('http://localhost/es')
+    })
+
+    it('should build one url per entry when "url" is a list', () => {
+      expect(createUrlFrom({hostname: 'http://localhost', pathOptions: {url: ['/es', '/en']}})).to.deep.equal([
+        'http://localhost/es',
+        'http://localhost/en'
+      ])
+    })
+  })
+
+  describe('resolveRouteOptions', () => {
+    it('should apply the requiredClassNames of the config to a route given as a plain string', () => {
+      const config = {requiredClassNames: ['.sui-Card']}
+
+      expect(resolveRouteOptions({pathOptions: '/es', config})).to.deep.equal({
+        requiredClassNames: ['.sui-Card'],
+        retries: 3
+      })
+    })
+
+    it('should apply the requiredClassNames of the config to a route that does not declare its own', () => {
+      const config = {requiredClassNames: ['.sui-Card']}
+
+      expect(resolveRouteOptions({pathOptions: {url: '/es'}, config}).requiredClassNames).to.deep.equal(['.sui-Card'])
+    })
+
+    it('should let a route override the requiredClassNames of the config', () => {
+      const config = {requiredClassNames: ['.sui-Card']}
+      const pathOptions = {url: '/es', requiredClassNames: ['.sui-List']}
+
+      expect(resolveRouteOptions({pathOptions, config}).requiredClassNames).to.deep.equal(['.sui-List'])
+    })
+
+    it('should let a route opt out of the requiredClassNames of the config with an empty list', () => {
+      const config = {requiredClassNames: ['.sui-Card']}
+      const pathOptions = {url: '/es', requiredClassNames: []}
+
+      expect(resolveRouteOptions({pathOptions, config}).requiredClassNames).to.deep.equal([])
+    })
+
+    it('should leave the requiredClassNames undefined when neither the config nor the route declares any', () => {
+      expect(resolveRouteOptions({pathOptions: {url: '/es'}}).requiredClassNames).to.equal(undefined)
+    })
+
+    it('should default the retries to 3', () => {
+      expect(resolveRouteOptions({pathOptions: '/es'}).retries).to.equal(3)
+    })
+
+    it('should apply the retries of the config', () => {
+      expect(resolveRouteOptions({pathOptions: '/es', config: {retries: 5}}).retries).to.equal(5)
+    })
+
+    it('should let a route override the retries of the config', () => {
+      const pathOptions = {url: '/es', retries: 1}
+
+      expect(resolveRouteOptions({pathOptions, config: {retries: 5}}).retries).to.equal(1)
+    })
+
+    it('should respect a route asking for 0 retries instead of falling back to the config', () => {
+      const pathOptions = {url: '/es', retries: 0}
+
+      expect(resolveRouteOptions({pathOptions, config: {retries: 5}}).retries).to.equal(0)
+    })
+
+    it('should not throw when the route has no options at all', () => {
+      expect(resolveRouteOptions({pathOptions: undefined, config: {requiredClassNames: ['.sui-Card']}})).to.deep.equal({
+        requiredClassNames: ['.sui-Card'],
+        retries: 3
+      })
+    })
+  })
+})