Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(deps): Don't use global variables for jsdom injection in scripts/package/node/core.js and core/utils/xml.ts #6764

Merged
merged 3 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 54 additions & 14 deletions core/utils/xml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,40 +14,80 @@
import * as goog from '../../closure/goog/goog.js';
goog.declareModuleId('Blockly.utils.xml');

import * as deprecation from './deprecation.js';


/**
* Namespace for Blockly's XML.
*
* @alias Blockly.utils.xml.NAME_SPACE
* Injected dependencies. By default these are just (and have the
* same types as) the corresponding DOM Window properties, but the
* Node.js wrapper for Blockly (see scripts/package/node/core.js)
* calls injectDependencies to supply implementations from the jsdom
* package instead.
*/
export const NAME_SPACE = 'https://developers.google.com/blockly/xml';
let {document, DOMParser, XMLSerializer} = globalThis;

/**
* 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
* Inject implementations of document, DOMParser and/or XMLSerializer
* to use instead of the default ones.
*
* Used by the Node.js wrapper for Blockly (see
* scripts/package/node/core.js) to supply implementations from the
* jsdom package instead.
*
* While they may be set individually, it is normally the case that
* all three will be sourced from the same JSDOM instance. They MUST
* at least come from the same copy of the jsdom package. (Typically
* this is hard to avoid satsifying this requirement, but it can be
* inadvertently violated by using webpack to build multiple bundles
* containing Blockly and jsdom, and then loading more than one into
* the same JavaScript runtime. See
* https://github.com/google/blockly-samples/pull/1452#issuecomment-1364442135
* for an example of how this happened.)
*
* @param dependencies Options object containing dependencies to set.
*/
let xmlDocument: Document = globalThis['document'];
export function injectDependencies(dependencies: {
document?: Document,
DOMParser?: typeof DOMParser,
XMLSerializer?: typeof XMLSerializer,
}) {
({
cpcallen marked this conversation as resolved.
Show resolved Hide resolved
// Default to existing value if option not supplied.
document = document,
DOMParser = DOMParser,
XMLSerializer = XMLSerializer,
} = dependencies);
}

/**
* Namespace for Blockly's XML.
*
* @alias Blockly.utils.xml.NAME_SPACE
*/
export const NAME_SPACE = 'https://developers.google.com/blockly/xml';

/**
* Get the document object to use for XML serialization.
*
* @returns The document object.
* @deprecated No longer provided by Blockly.
* @alias Blockly.utils.xml.getDocument
*/
export function getDocument(): Document {
return xmlDocument;
deprecation.warn('Blockly.utils.xml.getDocument', 'version 9', 'version 10');
BeksOmega marked this conversation as resolved.
Show resolved Hide resolved
return document;
}

/**
* Get the document object to use for XML serialization.
*
* @param document The document object to use.
* @param xmlDocument The document object to use.
* @deprecated No longer provided by Blockly.
* @alias Blockly.utils.xml.setDocument
*/
export function setDocument(document: Document) {
xmlDocument = document;
export function setDocument(xmlDocument: Document) {
deprecation.warn('Blockly.utils.xml.setDocument', 'version 9', 'version 10');
document = xmlDocument;
}

/**
Expand All @@ -58,7 +98,7 @@ export function setDocument(document: Document) {
* @alias Blockly.utils.xml.createElement
*/
export function createElement(tagName: string): Element {
return xmlDocument.createElementNS(NAME_SPACE, tagName);
return document.createElementNS(NAME_SPACE, tagName);
}

/**
Expand All @@ -69,7 +109,7 @@ export function createElement(tagName: string): Element {
* @alias Blockly.utils.xml.createTextNode
*/
export function createTextNode(text: string): Text {
return xmlDocument.createTextNode(text);
return document.createTextNode(text);
}

/**
Expand Down
6 changes: 1 addition & 5 deletions scripts/package/node/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,5 @@
if (typeof globalThis.document !== 'object') {
const {JSDOM} = require('jsdom');
const {window} = new JSDOM(`<!DOCTYPE html>`);
globalThis.DOMParser = window.DOMParser;
globalThis.XMLSerializer = window.XMLSerializer;
const xmlDocument = Blockly.utils.xml.textToDomDocument(
`<xml xmlns="${Blockly.utils.xml.NAME_SPACE}"></xml>`);
Blockly.utils.xml.setDocument(xmlDocument);
Blockly.utils.xml.injectDependencies(window);
}