From d202ae0201dbbfed8e52993ef63ef0853a520787 Mon Sep 17 00:00:00 2001 From: Christopher Allen Date: Tue, 14 Sep 2021 19:55:31 +0100 Subject: [PATCH] Don't monkey-patch Blocky.utils.xml.document (#5461) Use Blockly.utils.xml.setDocument instead. --- core/utils/xml.js | 36 +++++++++++++++++++++++----------- scripts/migration/renamings.js | 13 +++++++++--- scripts/package/node/core.js | 13 ++++++------ tests/deps.js | 2 +- 4 files changed, 42 insertions(+), 22 deletions(-) diff --git a/core/utils/xml.js b/core/utils/xml.js index d7e8d77cd..34dfdb170 100644 --- a/core/utils/xml.js +++ b/core/utils/xml.js @@ -19,6 +19,8 @@ goog.module('Blockly.utils.xml'); goog.module.declareLegacyNamespace(); +const {globalThis} = goog.require('Blockly.utils.global'); + /** * Namespace for Blockly's XML. @@ -27,19 +29,31 @@ const NAME_SPACE = 'https://developers.google.com/blockly/xml'; exports.NAME_SPACE = NAME_SPACE; /** - * Get the document object. This method is overridden in the Node.js build of - * Blockly. See gulpfile.js, package-blockly-node task. - * - * Note that this function is named getDocument so as to not shadow the - * global of the same name, but (for now) exported as .document to not - * break existing importers. - * + * The Document object to use. By default this is just document, but + * the Node.js build of Blockly (see scripts/package/node/core.js) + * calls setDocument to supply a Document implementation from the + * jsdom package instead. + * @type {!Document} + */ +let xmlDocument = globalThis.document; + +/** + * Get the document object to use for XML serialization. * @return {!Document} The document object. */ const getDocument = function() { - return document; + return xmlDocument; }; -exports.document = getDocument; +exports.getDocument = getDocument; + +/** + * Get the document object to use for XML serialization. + * @param {!Document} document The document object to use. + */ +const setDocument = function(document) { + xmlDocument = document; +}; +exports.setDocument = setDocument; /** * Create DOM element for XML. @@ -47,7 +61,7 @@ exports.document = getDocument; * @return {!Element} New DOM element. */ const createElement = function(tagName) { - return exports.document().createElementNS(NAME_SPACE, tagName); + return xmlDocument.createElementNS(NAME_SPACE, tagName); }; exports.createElement = createElement; @@ -57,7 +71,7 @@ exports.createElement = createElement; * @return {!Text} New DOM text node. */ const createTextNode = function(text) { - return exports.document().createTextNode(text); + return xmlDocument.createTextNode(text); }; exports.createTextNode = createTextNode; diff --git a/scripts/migration/renamings.js b/scripts/migration/renamings.js index 15c0a07f7..087964eae 100644 --- a/scripts/migration/renamings.js +++ b/scripts/migration/renamings.js @@ -32,7 +32,7 @@ const renamings = { export: 'conditionalBind', }, }, - } + }, }, '6.20210701.0': { 'Blockly': { @@ -70,14 +70,21 @@ const renamings = { 'Blockly.utils': { exports: { genUid: {module: 'Blockly.utils.idGenerator'}, - } + }, }, 'Blockly.utils.global': { export: 'globalThis', // Previous default export now named. }, 'Blockly.utils.IdGenerator': { module: 'Blockly.utils.idGenerator', - } + }, + 'Blockly.utils.xml': { + exports: { + // document was a function before, too - not a static property + // or get accessor. + document: {export: 'getDocument'}, + }, + }, }, }; diff --git a/scripts/package/node/core.js b/scripts/package/node/core.js index 21c2ac812..35e063b91 100644 --- a/scripts/package/node/core.js +++ b/scripts/package/node/core.js @@ -24,11 +24,10 @@ Blockly.setLocale = function (locale) { // XMLSerializer. const globalThis = Blockly.utils.global.globalThis; if (typeof globalThis.document !== 'object') { - globalThis.DOMParser = require('jsdom/lib/jsdom/living').DOMParser; - globalThis.XMLSerializer = require('jsdom/lib/jsdom/living').XMLSerializer; - var doc = Blockly.utils.xml.textToDomDocument( - ''); - Blockly.utils.xml.document = function() { - return doc; - }; + const jsdom = require('jsdom/lib/jsdom/living'); + globalThis.DOMParser = jsdom.DOMParser; + globalThis.XMLSerializer = jsdom.XMLSerializer; + const xmlDocument = Blockly.utils.xml.textToDomDocument( + ``); + Blockly.utils.xml.setDocument(xmlDocument); } diff --git a/tests/deps.js b/tests/deps.js index 75844ca38..dd99899f3 100644 --- a/tests/deps.js +++ b/tests/deps.js @@ -231,7 +231,7 @@ goog.addDependency('../../core/utils/svg.js', ['Blockly.utils.Svg'], [], {'lang' goog.addDependency('../../core/utils/svg_paths.js', ['Blockly.utils.svgPaths'], [], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../core/utils/toolbox.js', ['Blockly.utils.toolbox'], ['Blockly.Xml', 'Blockly.utils.userAgent'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../core/utils/useragent.js', ['Blockly.utils.userAgent'], ['Blockly.utils.global'], {'lang': 'es6', 'module': 'goog'}); -goog.addDependency('../../core/utils/xml.js', ['Blockly.utils.xml'], [], {'lang': 'es6', 'module': 'goog'}); +goog.addDependency('../../core/utils/xml.js', ['Blockly.utils.xml'], ['Blockly.utils.global'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../core/variable_map.js', ['Blockly.VariableMap'], ['Blockly.Events', 'Blockly.Events.VarDelete', 'Blockly.Events.VarRename', 'Blockly.Msg', 'Blockly.Names', 'Blockly.VariableModel', 'Blockly.dialog', 'Blockly.utils.idGenerator', 'Blockly.utils.object'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../core/variable_model.js', ['Blockly.VariableModel'], ['Blockly.Events', 'Blockly.Events.VarCreate', 'Blockly.utils.idGenerator'], {'lang': 'es6', 'module': 'goog'}); goog.addDependency('../../core/variables.js', ['Blockly.Variables'], ['Blockly.Blocks', 'Blockly.Msg', 'Blockly.VariableModel', 'Blockly.Xml', 'Blockly.dialog', 'Blockly.utils.xml'], {'lang': 'es6', 'module': 'goog'});