Skip to content

Commit

Permalink
fix: circular dependencies (#6281)
Browse files Browse the repository at this point in the history
* chore: fix circular dependencies w/ static workspace funcs

* remove preserved imports that aren't currently necessary (probably)

* fix circular dependency with workspaces and block using stub

* fix dependency between variables and xml by moving function to utils

* add stub for trashcan as well

* fix line endings from rebase

* fix goog/base order

* add trashcan patch

* fix: types of compose and decompose in block

* fix: workspace naming in toolbox

* chore: add jsdoc

* chore: restore registry comments to better positions

* chore: remove implementations in goog.js

* chore: fix types of stubs

* chore: remove added AnyDuringMigration casts

* chore: remove modifications to xml and variables

* chore: format

* chore: remove event requirements in workspace comments

* chore: fix circular dependency with xml and workspace comments

* fixup remove ContextMenu import

* chore: fix dependency between mutator and workspace

* chore: break circular dependency between names and procedures

* chore: get tests to run?

* chore: pr comments'

* chore: fix stubbing field registry fromJson

* chore: fix spying on fire

* chore: fix stubbing parts of connection checker

* chore: fix stubbing dialog

* chore: fix stubbing style

* chore: fix spying on duplicate

* chore: fix stubbing variables

* chore: fix stubbing copy

* chore: fix stubbing in workspace

* chore: remove unnecessary stubs

* chore: fix formatting

* chore: fix other formatting

* chore: add backwards compatible static properties to workspace

* chore: move static type properties

* chore: move and comment stubs

* chore: add newlines at EOF

* chore: improve errors for monkey patched functions

* chore: update comment with a pointer to the doc

* chore: update comment with a pointer to the doc

* chore: format
  • Loading branch information
BeksOmega authored Jul 28, 2022
1 parent aa61f2d commit 9d5dcc6
Show file tree
Hide file tree
Showing 72 changed files with 525 additions and 335 deletions.
4 changes: 2 additions & 2 deletions closure/goog/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -617,8 +617,8 @@ goog.declareModuleId = function(namespace) {
'within an ES6 module');
}
if (goog.moduleLoaderState_ && goog.moduleLoaderState_.moduleName) {
throw new Error(
'goog.declareModuleId may only be called once per module.');
// throw new Error(
// 'goog.declareModuleId may only be called once per module.');
}
if (namespace in goog.loadedModules_) {
throw new Error(
Expand Down
83 changes: 44 additions & 39 deletions closure/goog/goog.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,48 +36,53 @@
* goog.require calls.
*/

export const global = goog.global;
export const require = goog.require;
export const define = goog.define;
export const DEBUG = goog.DEBUG;
export const LOCALE = goog.LOCALE;
export const TRUSTED_SITE = goog.TRUSTED_SITE;
export const DISALLOW_TEST_ONLY_CODE = goog.DISALLOW_TEST_ONLY_CODE;
export const getGoogModule = goog.module.get;
export const setTestOnly = goog.setTestOnly;
export const forwardDeclare = goog.forwardDeclare;
export const getObjectByName = goog.getObjectByName;
export const basePath = goog.basePath;
export const addSingletonGetter = goog.addSingletonGetter;
export const typeOf = goog.typeOf;
export const isArrayLike = goog.isArrayLike;
export const isDateLike = goog.isDateLike;
export const isObject = goog.isObject;
export const getUid = goog.getUid;
export const hasUid = goog.hasUid;
export const removeUid = goog.removeUid;
export const now = Date.now;
export const globalEval = goog.globalEval;
export const getCssName = goog.getCssName;
export const setCssNameMapping = goog.setCssNameMapping;
export const getMsg = goog.getMsg;
export const getMsgWithFallback = goog.getMsgWithFallback;
export const exportSymbol = goog.exportSymbol;
export const exportProperty = goog.exportProperty;
export const abstractMethod = goog.abstractMethod;
export const cloneObject = goog.cloneObject;
export const bind = goog.bind;
export const partial = goog.partial;
export const inherits = goog.inherits;
export const scope = goog.scope;
export const defineClass = goog.defineClass;
export const declareModuleId = goog.declareModuleId;
export const global = globalThis;
// export const require = goog.require;
// export const define = goog.define;
// export const DEBUG = goog.DEBUG;
// export const LOCALE = goog.LOCALE;
// export const TRUSTED_SITE = goog.TRUSTED_SITE;
// export const DISALLOW_TEST_ONLY_CODE = goog.DISALLOW_TEST_ONLY_CODE;
// export const getGoogModule = goog.module.get;
// export const setTestOnly = goog.setTestOnly;
// export const forwardDeclare = goog.forwardDeclare;
// export const getObjectByName = goog.getObjectByName;
// export const basePath = goog.basePath;
// export const addSingletonGetter = goog.addSingletonGetter;
// export const typeOf = goog.typeOf;
// export const isArrayLike = goog.isArrayLike;
// export const isDateLike = goog.isDateLike;
// export const isObject = goog.isObject;
// export const getUid = goog.getUid;
// export const hasUid = goog.hasUid;
// export const removeUid = goog.removeUid;
// export const now = Date.now;
// export const globalEval = goog.globalEval;
// export const getCssName = goog.getCssName;
// export const setCssNameMapping = goog.setCssNameMapping;
// export const getMsg = goog.getMsg;
// export const getMsgWithFallback = goog.getMsgWithFallback;
// export const exportSymbol = goog.exportSymbol;
// export const exportProperty = goog.exportProperty;
// export const abstractMethod = goog.abstractMethod;
// export const cloneObject = goog.cloneObject;
// export const bind = goog.bind;
// export const partial = goog.partial;
// export const inherits = goog.inherits;
// export const scope = goog.scope;
// export const defineClass = goog.defineClass;
export const declareModuleId = function(namespace) {
if (window.goog && window.goog.declareModuleId) {
window.goog.declareModuleId.call(this, namespace);
}
};


// Export select properties of module. Do not export the function itself or
// goog.module.declareLegacyNamespace.
export const module = {
get: goog.module.get,
};
// export const module = {
// get: goog.module.get,
// };

// Omissions include:
// goog.ENABLE_DEBUG_LOADER - define only used in base.
Expand Down
4 changes: 2 additions & 2 deletions core/block_svg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ goog.declareModuleId('Blockly.BlockSvg');

/* eslint-disable-next-line no-unused-vars */
// Unused import preserved for side-effects. Remove if unneeded.
import './theme.js';
// import './theme.js';
// Unused import preserved for side-effects. Remove if unneeded.
import './events/events_selected.js';
// Unused import preserved for side-effects. Remove if unneeded.
import './touch.js';
// import './touch.js';

import {Block} from './block.js';
import * as blockAnimations from './block_animations.js';
Expand Down
53 changes: 53 additions & 0 deletions core/blockly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,59 @@ export const VARIABLE_DYNAMIC_CATEGORY_NAME: string =
export const PROCEDURE_CATEGORY_NAME: string =
(Procedures as AnyDuringMigration).CATEGORY_NAME;

// Context for why we need to monkey-patch in these functions (internal):
// https://docs.google.com/document/d/1MbO0LEA-pAyx1ErGLJnyUqTLrcYTo-5zga9qplnxeXo/edit?usp=sharing&resourcekey=0-5h_32-i-dHwHjf_9KYEVKg

// clang-format off
Workspace.prototype.newBlock =
function(prototypeName: string, opt_id?: string): Block {
return new Block(this, prototypeName, opt_id);
}

WorkspaceSvg.prototype.newBlock =
function(prototypeName: string, opt_id?: string): BlockSvg {
return new BlockSvg(this, prototypeName, opt_id);
}

WorkspaceSvg.newTrashcan = function(workspace: WorkspaceSvg): Trashcan {
return new Trashcan(workspace);
}

WorkspaceCommentSvg.prototype.showContextMenu =
function(this: WorkspaceCommentSvg, e: Event) {
if (this.workspace.options.readOnly) {
return;
}
// Save the current workspace comment in a variable for use in closures.
const comment = this;
const menuOptions = [];

if (this.isDeletable() && this.isMovable()) {
menuOptions.push(ContextMenu.commentDuplicateOption(comment));
menuOptions.push(ContextMenu.commentDeleteOption(comment));
}

ContextMenu.show(e, menuOptions, this.RTL);
}

Mutator.prototype.newWorkspaceSvg =
function(options: Options): WorkspaceSvg {
return new WorkspaceSvg(options);
}

Names.prototype.populateProcedures =
function(this: Names, workspace: Workspace) {
let procedures = Procedures.allProcedures(workspace);
// Flatten the return vs no-return procedure lists.
let flattenedProcedures: AnyDuringMigration[][] =
procedures[0].concat(procedures[1]);
for (let i = 0; i < flattenedProcedures.length; i++) {
this.getName(flattenedProcedures[i][0], Names.NameType.PROCEDURE);
}
}
// clang-format on


// Re-export submodules that no longer declareLegacyNamespace.
export {browserEvents};
export {ContextMenu};
Expand Down
4 changes: 2 additions & 2 deletions core/bubble.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ goog.declareModuleId('Blockly.Bubble');

/* eslint-disable-next-line no-unused-vars */
// Unused import preserved for side-effects. Remove if unneeded.
import './metrics_manager.js';
// import './metrics_manager.js';
// Unused import preserved for side-effects. Remove if unneeded.
import './workspace.js';
// import './workspace.js';

import type {BlockDragSurfaceSvg} from './block_drag_surface.js';
import type {BlockSvg} from './block_svg.js';
Expand Down
4 changes: 2 additions & 2 deletions core/bubble_dragger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ import * as goog from '../closure/goog/goog.js';
goog.declareModuleId('Blockly.BubbleDragger');

// Unused import preserved for side-effects. Remove if unneeded.
import './bubble.js';
// import './bubble.js';
// Unused import preserved for side-effects. Remove if unneeded.
import './constants.js';
// import './constants.js';

import type {BlockDragSurfaceSvg} from './block_drag_surface.js';
import {ComponentManager} from './component_manager.js';
Expand Down
19 changes: 19 additions & 0 deletions core/clipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ let copyData: CopyData|null = null;
* @internal
*/
export function copy(toCopy: ICopyable) {
TEST_ONLY.copyInternal(toCopy);
}

/**
* Private version of copy for stubbing in tests.
*/
function copyInternal(toCopy: ICopyable) {
copyData = toCopy.toCopyData();
}

Expand Down Expand Up @@ -63,9 +70,21 @@ export function paste(): ICopyable|null {
* @internal
*/
export function duplicate(toDuplicate: ICopyable): ICopyable|null {
return TEST_ONLY.duplicateInternal(toDuplicate);
}

/**
* Private version of duplicate for stubbing in tests.
*/
function duplicateInternal(toDuplicate: ICopyable): ICopyable|null {
const oldCopyData = copyData;
copy(toDuplicate);
const pastedThing = toDuplicate.toCopyData().source.paste(copyData!.saveInfo);
copyData = oldCopyData;
return pastedThing;
}

export const TEST_ONLY = {
duplicateInternal,
copyInternal,
}
6 changes: 3 additions & 3 deletions core/comment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,16 @@ goog.declareModuleId('Blockly.Comment');

/* eslint-disable-next-line no-unused-vars */
// Unused import preserved for side-effects. Remove if unneeded.
import './block.js';
// import './block.js';
/* eslint-disable-next-line no-unused-vars */
// Unused import preserved for side-effects. Remove if unneeded.
import './workspace_svg.js';
// import './workspace_svg.js';
// Unused import preserved for side-effects. Remove if unneeded.
import './events/events_block_change.js';
// Unused import preserved for side-effects. Remove if unneeded.
import './events/events_bubble_open.js';
// Unused import preserved for side-effects. Remove if unneeded.
import './warning.js';
// import './warning.js';

import type {CommentModel} from './block.js';
import type {BlockSvg} from './block_svg.js';
Expand Down
42 changes: 42 additions & 0 deletions core/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,39 @@ import type {Workspace} from './workspace.js';
import type {WorkspaceSvg} from './workspace_svg.js';


/** Database of all workspaces. */
const WorkspaceDB_ = Object.create(null);


/**
* Find the workspace with the specified ID.
* @param id ID of workspace to find.
* @return The sought after workspace or null if not found.
*/
export function getWorkspaceById(id: string): Workspace|null {
return WorkspaceDB_[id] || null;
}

/**
* Find all workspaces.
* @return Array of workspaces.
*/
export function getAllWorkspaces(): Workspace[] {
const workspaces = [];
for (const workspaceId in WorkspaceDB_) {
workspaces.push(WorkspaceDB_[workspaceId]);
}
return workspaces;
}

export function registerWorkspace(workspace: Workspace) {
WorkspaceDB_[workspace.id] = workspace;
}

export function unregisterWorkpace(workspace: Workspace) {
delete WorkspaceDB_[workspace.id];
}

/**
* The main workspace most recently used.
* Set by Blockly.WorkspaceSvg.prototype.markFocused
Expand Down Expand Up @@ -196,6 +229,13 @@ function jsonInitFactory(jsonDef: AnyDuringMigration): () => void {
* @alias Blockly.common.defineBlocksWithJsonArray
*/
export function defineBlocksWithJsonArray(jsonArray: AnyDuringMigration[]) {
TEST_ONLY.defineBlocksWithJsonArrayInternal(jsonArray);
}

/**
* Private version of defineBlocksWithJsonArray for stubbing in tests.
*/
function defineBlocksWithJsonArrayInternal(jsonArray: AnyDuringMigration[]) {
defineBlocks(createBlockDefinitionsFromJsonArray(jsonArray));
}

Expand Down Expand Up @@ -245,3 +285,5 @@ export function defineBlocks(blocks: {[key: string]: BlockDefinition}) {
Blocks[type] = definition;
}
}

export const TEST_ONLY = {defineBlocksWithJsonArrayInternal};
2 changes: 1 addition & 1 deletion core/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import * as goog from '../closure/goog/goog.js';
goog.declareModuleId('Blockly.Connection');

// Unused import preserved for side-effects. Remove if unneeded.
import './constants.js';
// import './constants.js';

import type {Block} from './block.js';
import {ConnectionType} from './connection_type.js';
Expand Down
12 changes: 6 additions & 6 deletions core/connection_db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import * as goog from '../closure/goog/goog.js';
goog.declareModuleId('Blockly.ConnectionDB');

// Unused import preserved for side-effects. Remove if unneeded.
import './constants.js';
// import './constants.js';

import {ConnectionType} from './connection_type.js';
import type {IConnectionChecker} from './interfaces/i_connection_checker.js';
Expand All @@ -39,10 +39,10 @@ export class ConnectionDB {
private readonly connections_: RenderedConnection[] = [];

/**
* @param checker The workspace's connection type checker, used to decide if
* connections are valid during a drag.
* @param connectionChecker The workspace's connection type checker, used to
* decide if connections are valid during a drag.
*/
constructor(private readonly checker: IConnectionChecker) {}
constructor(private readonly connectionChecker: IConnectionChecker) {}

/**
* Add a connection to the database. Should not already exist in the database.
Expand Down Expand Up @@ -248,7 +248,7 @@ export class ConnectionDB {
let pointerMin = closestIndex - 1;
while (pointerMin >= 0 && this.isInYRange_(pointerMin, conn.y, maxRadius)) {
temp = this.connections_[pointerMin];
if (this.checker.canConnect(conn, temp, true, bestRadius)) {
if (this.connectionChecker.canConnect(conn, temp, true, bestRadius)) {
bestConnection = temp;
// AnyDuringMigration because: Argument of type 'RenderedConnection' is
// not assignable to parameter of type 'Connection'.
Expand All @@ -261,7 +261,7 @@ export class ConnectionDB {
while (pointerMax < this.connections_.length &&
this.isInYRange_(pointerMax, conn.y, maxRadius)) {
temp = this.connections_[pointerMax];
if (this.checker.canConnect(conn, temp, true, bestRadius)) {
if (this.connectionChecker.canConnect(conn, temp, true, bestRadius)) {
bestConnection = temp;
// AnyDuringMigration because: Argument of type 'RenderedConnection' is
// not assignable to parameter of type 'Connection'.
Expand Down
13 changes: 13 additions & 0 deletions core/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,18 @@ export function setAlert(
*/
export function confirm(
message: string, callback: (p1: boolean) => AnyDuringMigration) {
TEST_ONLY.confirmInternal(message, callback);
}

/**
* Private version of confirm for stubbing in tests.
*/
function confirmInternal(
message: string, callback: (p1: boolean) => AnyDuringMigration) {
confirmImplementation(message, callback);
}


/**
* Sets the function to be run when Blockly.dialog.confirm() is called.
* @param confirmFunction The function to be run.
Expand Down Expand Up @@ -112,3 +121,7 @@ export function setPrompt(
AnyDuringMigration) {
promptImplementation = promptFunction;
}

export const TEST_ONLY = {
confirmInternal,
}
Loading

0 comments on commit 9d5dcc6

Please sign in to comment.