RS-22108: Wait for fonts before laying out the heatmap

.github/workflows/js-tests.yaml

@@ -0,0 +1,56 @@
+name: JS tests
+
+# Modelled on rhtmlCombinedScatter's js-tests.yaml, minus its visual job. The visual suite is not
+# runnable here yet: this repo is on rhtmlBuildUtils 7.1.1, whose puppeteer 3.3.0 ships Chromium 83,
+# whose --env is whitelisted to local/travis, whose --acceptNewSnapshots defaults to true so a missing
+# baseline is silently written and passes, and whose jest-image-snapshot swallows mismatches per test.
+# The committed baselines were also generated on Windows, so they would all fail on ubuntu at the
+# 0.0001% threshold. Adding it is a rhtmlBuildUtils upgrade, not a workflow file.
+
+on:
+  push:
+  workflow_dispatch:
+
+# One in-flight run per branch; a new push supersedes the previous one.
+concurrency:
+  group: js-tests-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  unit:
+    name: Unit tests and lint
+    runs-on: ubuntu-24.04
+    timeout-minutes: 20
+    env:
+      # No browser is launched here, so skip the Chromium download. puppeteer 3.3.0 honours this.
+      PUPPETEER_SKIP_DOWNLOAD: 'true'
+    steps:
+      - uses: actions/checkout@v7
+
+      - uses: actions/setup-node@v7
+        with:
+          node-version: 22
+          cache: npm
+
+      # The preinstall hook self-skips when CI is true, which the runner sets.
+      - name: Install dependencies
+        id: install
+        run: npm ci
+
+      # Every step below runs even if an earlier one failed, so one failure does not hide the rest.
+      # Gated on the install succeeding, so a broken npm ci does not cascade.
+      - name: Lint
+        if: ${{ !cancelled() && steps.install.outcome == 'success' }}
+        run: npx gulp lint
+
+      - name: Unit tests
+        if: ${{ !cancelled() && steps.install.outcome == 'success' }}
+        run: npx gulp testSpecs
+
+      # Deliberately not `gulp build`: its `clean` step deletes the tracked man/ and R/, which only
+      # makeDocs can rebuild, and makeDocs needs R and devtools. This task is the compile check we
+      # want, and it earns its place in this repo because inst/htmlwidgets/ is committed, so it
+      # catches a source change pushed without a rebuild.
+      - name: Compile widget bundle
+        if: ${{ !cancelled() && steps.install.outcome == 'success' }}
+        run: npx gulp compileWidgetEntryPoint

theSrc/scripts/heatmapOuter.jest.test.js

@@ -0,0 +1,79 @@
+jest.mock('./lib/heatmapcore/heatmapcore', () => function Heatmap () {})
+
+const heatmapOuter = require('./heatmapOuter')
+
+// The real Image decodes a data uri asynchronously. This stands in for it so that a test decides
+// when, and whether, the load succeeds
+class ControllableImage {
+  constructor () {
+    ControllableImage.instances.push(this)
+    this.onload = null
+    this.onerror = null
+  }
+
+  set src (uri) { this._src = uri }
+
+  static get latest () { return ControllableImage.instances[ControllableImage.instances.length - 1] }
+}
+ControllableImage.instances = []
+
+const config = () => ({
+  options: { logLevel: 'silent' },
+  image: 'data:image/png;base64,notarealimage',
+  matrix: { dim: [1, 1], data: [], cells_to_hide: [], cellnote_in_cell: [] },
+  rows: null,
+  cols: null,
+})
+
+const statusOf = element => element.getAttribute('rhtmlwidget-status')
+
+describe('heatmapOuter', () => {
+  let element = null
+
+  beforeEach(() => {
+    ControllableImage.instances = []
+    global.Image = ControllableImage
+    element = document.createElement('div')
+    document.body.appendChild(element)
+  })
+
+  afterEach(() => {
+    document.body.removeChild(element)
+    delete global.Image
+  })
+
+  test('claims loading before any of the asynchronous work', () => {
+    heatmapOuter(element, config())
+
+    expect(statusOf(element)).toEqual('loading')
+  })
+
+  test('reports ready when the image cannot be loaded', async () => {
+    const rendering = heatmapOuter(element, config())
+    ControllableImage.latest.onerror()
+
+    await expect(rendering).rejects.toThrow('failed to load the heatmap colour image')
+    expect(statusOf(element)).toEqual('ready')
+  })
+
+  test('leaves the status alone when the render it belongs to has been superseded', async () => {
+    const rendering = heatmapOuter(element, config())
+    const supersededImage = ControllableImage.latest
+
+    // What the factory does on a resize: discard the markup and render again from scratch
+    element.innerHTML = ''
+    heatmapOuter(element, config())
+
+    supersededImage.onerror()
+
+    await expect(rendering).rejects.toThrow('failed to load the heatmap colour image')
+    expect(statusOf(element)).toEqual('loading')
+  })
+
+  test('reports ready when the render fails synchronously', () => {
+    element.appendChild = () => { throw new Error('cannot append') }
+
+    expect(() => heatmapOuter(element, config())).toThrow('cannot append')
+    expect(statusOf(element)).toEqual('ready')
+  })
+})

theSrc/scripts/heatmapOuter.js

@@ -1,9 +1,9 @@
 /* global Image */
 
-import _ from 'lodash'
-import d3 from 'd3'
-import * as rootLog from 'loglevel'
-
+const _ = require('lodash')
+const d3 = require('d3')
+const rootLog = require('loglevel')
+const { waitForFonts } = require('./lib/fonts')
 const Heatmap = require('./lib/heatmapcore/heatmapcore')
 
 let uniqueInstanceCount = 0
@@ -16,32 +16,63 @@ module.exports = function (element, config) {
 
   _initLogger(options.logLevel)
 
-  const { width, height } = getContainerDimensions(_.has(element, 'length') ? element[0] : element)
+  const rootElement = _.has(element, 'length') ? element[0] : element
+  const { width, height } = getContainerDimensions(rootElement)
   const uniqueClass = `heatmap-${uniqueId()}`
 
-  d3.select(element)
-    .append('svg')
-    .attr('class', `svgContent ${uniqueClass}`)
-    .attr('width', width)
-    .attr('height', height)
-
-  loadImage(image)
-    .then(({ imgData, width, height }) => processImageData({ imgData, width, height, matrix, cellNotes: options.shownote_in_cell }))
-    .then(merged => {
-      matrix.merged = merged
-      return new Heatmap({
-        selector: `.svgContent.${uniqueClass}`,
-        options,
-        matrix,
-        dendrogramRows: rows,
-        dendrogramColumns: cols,
-        width,
-        height,
+  // The status must be claimed before the async work below, otherwise Displayr can treat the
+  // widget as rendered and screenshot it while it is still waiting on fonts or on the image data
+  rootElement.setAttribute('rhtmlwidget-status', 'loading')
+
+  // A resize renders from scratch and does not cancel the render it interrupts, so a chain can
+  // still be in flight after its svg has been discarded. Only the render whose svg is still in
+  // the container may report the status, or a stale chain marks a newer, unfinished chart ready
+  const isCurrentRender = () => Boolean(rootElement.querySelector(`.svgContent.${uniqueClass}`))
+
+  try {
+    d3.select(element)
+      .append('svg')
+      .attr('class', `svgContent ${uniqueClass}`)
+      .attr('width', width)
+      .attr('height', height)
+
+    // Fonts are waited on alongside the image load, not after it, so this costs no extra time
+    // when the fonts are already available. The chain is returned so that tests can await it;
+    // nothing consumes it in production, which leaves a failed render to Displayr's bug catcher
+    return Promise.all([loadImage(image), waitForFonts(options)])
+      .then(([{ imgData, width, height }]) => processImageData({ imgData, width, height, matrix, cellNotes: options.shownote_in_cell }))
+      .then(merged => {
+        matrix.merged = merged
+        return new Heatmap({
+          selector: `.svgContent.${uniqueClass}`,
+          options,
+          matrix,
+          dendrogramRows: rows,
+          dendrogramColumns: cols,
+          width,
+          height,
+        })
       })
-    })
-    .catch(error => {
-      throw error
-    })
+      .then(() => {
+        if (isCurrentRender()) {
+          rootElement.setAttribute('rhtmlwidget-status', 'ready')
+        }
+      })
+      .catch(error => {
+        // The status must not be left as loading, or Displayr waits on a chart that will never
+        // arrive, which for an image export means waiting out its screenshot timeout
+        if (isCurrentRender()) {
+          rootElement.setAttribute('rhtmlwidget-status', 'ready')
+        }
+        throw error
+      })
+  } catch (error) {
+    // A synchronous failure here would otherwise leave the status claimed with no chain to
+    // release it. No newer render can have started while this one was still synchronous, so
+    // this is the current render and the status is reported without checking
+    rootElement.setAttribute('rhtmlwidget-status', 'ready')
+    throw error
+  }
 }
 
 function _initLogger (loggerSettings = 'info') {
@@ -67,9 +98,11 @@ function getLoggerNames () {
 }
 
 function loadImage (uri) {
-  // TODO add better img load fail -> reject wiring here
   return new Promise((resolve, reject) => {
     var img = new Image()
+    // Without this the promise never settles on a corrupt or malformed uri, and the render is
+    // left with no way to report that it is finished
+    img.onerror = () => reject(new Error('failed to load the heatmap colour image'))
     img.onload = function () {
       // Save size
       const width = img.width

theSrc/scripts/lib/fonts.jest.test.js

@@ -0,0 +1,128 @@
+const { fontFamiliesInUse, waitForFonts } = require('./fonts.js')
+
+describe('fontFamiliesInUse', () => {
+  test('collects every font family option', () => {
+    expect(fontFamiliesInUse({
+      title_font_family: 'Circular',
+      xaxis_font_family: 'Open Sans',
+      xaxis_font_size: 15,
+      yaxis_hidden: false,
+    })).toEqual(['Circular', 'Open Sans'])
+  })
+
+  test('deduplicates, trims, and drops values that are not usable font families', () => {
+    expect(fontFamiliesInUse({
+      title_font_family: 'Circular',
+      subtitle_font_family: ' Circular ',
+      footer_font_family: '',
+      legend_font_family: null,
+      cell_font_family: 12,
+    })).toEqual(['Circular'])
+  })
+
+  test('returns nothing when no font families are configured', () => {
+    expect(fontFamiliesInUse({ xaxis_font_size: 15 })).toEqual([])
+    expect(fontFamiliesInUse({})).toEqual([])
+  })
+})
+
+describe('waitForFonts', () => {
+  // document cannot be replaced wholesale under jsdom, so only the font set is stubbed, and
+  // a document is only invented when the test environment provides none
+  const documentWasInvented = (typeof document === 'undefined')
+  const originalFontSet = documentWasInvented ? undefined : document.fonts
+
+  const withFontSet = (fontSet) => {
+    if (documentWasInvented) {
+      global.document = {}
+    }
+    document.fonts = fontSet
+  }
+
+  afterEach(() => {
+    if (documentWasInvented) {
+      delete global.document
+    } else {
+      document.fonts = originalFontSet
+    }
+  })
+
+  test('requests each configured family in normal and bold, then waits on the font set', async () => {
+    const requested = []
+    let readyHasResolved = false
+    withFontSet({
+      load: (fontSpecification) => { requested.push(fontSpecification); return Promise.resolve([]) },
+      ready: Promise.resolve().then(() => { readyHasResolved = true }),
+    })
+
+    await waitForFonts({ title_font_family: 'Circular', xaxis_font_family: 'Circular' })
+
+    expect(requested).toEqual(['12px "Circular"', 'bold 12px "Circular"'])
+    expect(readyHasResolved).toBe(true)
+  })
+
+  test('quotes the family so a multi word name stays a parseable font shorthand', async () => {
+    const requested = []
+    withFontSet({
+      load: (fontSpecification) => { requested.push(fontSpecification); return Promise.resolve([]) },
+      ready: Promise.resolve(),
+    })
+
+    await waitForFonts({ title_font_family: 'Open Sans' })
+
+    expect(requested).toEqual(['12px "Open Sans"', 'bold 12px "Open Sans"'])
+  })
+
+  test('resolves when a font cannot be loaded', async () => {
+    withFontSet({
+      load: () => Promise.reject(new Error('no such font')),
+      ready: Promise.resolve(),
+    })
+
+    await expect(waitForFonts({ title_font_family: 'Circular' })).resolves.toBeUndefined()
+  })
+
+  test('resolves when loading a font throws synchronously, as Blink does on an unparseable shorthand', async () => {
+    withFontSet({
+      load: () => { throw new Error('Could not resolve as a font') },
+      ready: Promise.resolve(),
+    })
+
+    await expect(waitForFonts({ title_font_family: 'a "quoted" name' })).resolves.toBeUndefined()
+  })
+
+  test('resolves when the font set has no load method', async () => {
+    withFontSet({ ready: Promise.resolve() })
+
+    await expect(waitForFonts({ title_font_family: 'Circular' })).resolves.toBeUndefined()
+  })
+
+  test('resolves when the font set rejects, which the spec forbids but a shim may do', async () => {
+    withFontSet({
+      load: () => Promise.resolve([]),
+      ready: Promise.reject(new Error('not a conforming font set')),
+    })
+
+    await expect(waitForFonts({ title_font_family: 'Circular' })).resolves.toBeUndefined()
+  })
+
+  test('resolves without waiting for a font set the browser does not provide', async () => {
+    withFontSet(undefined)
+
+    await expect(waitForFonts({ title_font_family: 'Circular' })).resolves.toBeUndefined()
+  })
+
+  test('stops waiting on a font set that never becomes ready', async () => {
+    jest.useFakeTimers()
+    try {
+      withFontSet({ load: () => Promise.resolve([]), ready: new Promise(() => {}) })
+
+      const waiting = waitForFonts({ title_font_family: 'Circular' })
+      jest.advanceTimersByTime(3000)
+
+      await expect(waiting).resolves.toBeUndefined()
+    } finally {
+      jest.useRealTimers()
+    }
+  })
+})