From 77f7d1453c6fd212554687fd446c2be6555e2f61 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 27 Jan 2022 10:44:20 +0000 Subject: [PATCH] Replace domdiff library with morphdom We added domdiff to replace the DiffDOM library here: https://github.com/alphagov/notifications-admin/commit/87f54d1e886f5c45ef7f12d88ef988c3f9343641 DiffDOM had updated its code to be written to the ECMAScript 6 (ES6) standard and so needed extra work to work with the older browsers in our support matrix. This was recorded as an issue here: https://www.pivotaltracker.com/n/projects/1443052/stories/165380360 Domdiff didn't work (see below for more details) so this replaces it with the morphdom library. Morphdom supports the same browsers as us and is relied on by a range of large open source projects: https://github.com/patrick-steele-idem/morphdom#what-projects-are-using-morphdom It was tricky to find alternatives to DiffDOM so if we have to source alternatives in future, other options could be: - https://github.com/choojs/nanomorph - https://diffhtml.org/index.html (using its outerHTML method) Why domdiff didn't work Turns out that domdiff was replacing the page HTML with the HTML from the AJAX response every time, not just when they differed. This isn't a bug. Domdiff is bare bones enough that it compares old DOM nodes to new DOM nodes with ===. With our code, this always results to false because our new nodes are made from HTML strings from AJAX response so are never the same node as the old one. --- app/assets/javascripts/modules/all.mjs | 16 ++++++++++++++-- app/assets/javascripts/updateContent.js | 17 +++++------------ gulpfile.js | 1 - package.json | 2 +- tests/javascripts/updateContent.test.js | 13 ++++++------- 5 files changed, 26 insertions(+), 23 deletions(-) diff --git a/app/assets/javascripts/modules/all.mjs b/app/assets/javascripts/modules/all.mjs index a62daad45..33af699f4 100644 --- a/app/assets/javascripts/modules/all.mjs +++ b/app/assets/javascripts/modules/all.mjs @@ -6,11 +6,16 @@ // // Exported items will be added to the window.GOVUK namespace. // For example, `export { Frontend }` will assign `Frontend` to `window.Frontend` + +// GOVUK Frontend modules import Header from 'govuk-frontend/components/header/header'; import Details from 'govuk-frontend/components/details/details'; import Button from 'govuk-frontend/components/button/button'; import Radios from 'govuk-frontend/components/radios/radios'; +// Modules from 3rd party vendors +import morphdom from 'morphdom'; + /** * TODO: Ideally this would be a NodeList.prototype.forEach polyfill * This seems to fail in IE8, requires more investigation. @@ -62,6 +67,13 @@ var Frontend = { "initAll": initAll } -export { - Frontend +var vendor = { + "morphdom": morphdom +} + +// The exported object will be assigned to window.GOVUK in our production code +// (bundled into an IIFE by RollupJS) +export { + Frontend, + vendor } diff --git a/app/assets/javascripts/updateContent.js b/app/assets/javascripts/updateContent.js index 85cfa61e5..2e4bc4758 100644 --- a/app/assets/javascripts/updateContent.js +++ b/app/assets/javascripts/updateContent.js @@ -2,6 +2,7 @@ "use strict"; var queues = {}; + var morphdom = global.GOVUK.vendor.morphdom; var defaultInterval = 2000; var interval = 0; @@ -10,18 +11,10 @@ 1000 )); - var getRenderer = $component => { - var key = $component.data('key'); // use closure to retain key when component is replaced - return response => { - $component = $( - global.domdiff( - $component.parent().get(0), - [$component.get(0)], - [$(response[key]).get(0)] - )[0] - ); - }; - }; + var getRenderer = $component => response => morphdom( + $component.get(0), + $(response[$component.data('key')]).get(0) + ); var getQueue = resource => ( queues[resource] = queues[resource] || [] diff --git a/gulpfile.js b/gulpfile.js index 68964ea9d..fa1128fc1 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -146,7 +146,6 @@ const javascripts = () => { paths.npm + 'hogan.js/dist/hogan-3.0.2.js', paths.npm + 'jquery/dist/jquery.min.js', paths.npm + 'query-command-supported/dist/queryCommandSupported.min.js', - paths.npm + 'domdiff/min.js', paths.npm + 'timeago/jquery.timeago.js', paths.npm + 'textarea-caret/index.js', paths.npm + 'cbor-js/cbor.js' diff --git a/package.json b/package.json index 726b41cfd..418b73fae 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,6 @@ "@babel/preset-env": "7.4.2", "cbor-js": "0.1.0", "del": "5.1.0", - "domdiff": "2.2.2", "govuk_frontend_toolkit": "8.1.0", "govuk-elements-sass": "3.1.2", "govuk-frontend": "2.13.0", @@ -39,6 +38,7 @@ "hogan": "1.0.2", "jquery": "3.5.0", "leaflet": "1.6.0", + "morphdom": "2.6.1", "query-command-supported": "1.0.0", "rollup": "1.23.1", "sass": "1.32.7", diff --git a/tests/javascripts/updateContent.test.js b/tests/javascripts/updateContent.test.js index c1496b4b7..d965906b8 100644 --- a/tests/javascripts/updateContent.test.js +++ b/tests/javascripts/updateContent.test.js @@ -31,13 +31,12 @@ beforeAll(() => { $.ajax.mockImplementation(() => jqueryAJAXReturnObj); - // using require to execute the version we use in our our frontend build here can't add - // the domdiff variable to this scope like it does when executed in browsers because - // that version doesn't export it - // we use CommonJS version instead because it does (as the default property) - // see https://nodejs.org/en/knowledge/getting-started/what-is-require/ for more info - // also, we're not a browser so we need to manually attach domdiff to window - window.domdiff = require('domdiff/cjs').default; + // RollupJS assigns our bundled module code, including morphdom, to window.GOVUK. + // morphdom is assigned to its vendor property so we need to copy that here for the updateContent + // code to pick it up. + window.GOVUK.vendor = { + morphdom: require('morphdom') + }; require('../../app/assets/javascripts/updateContent.js'); });