Throw Errors instead of using goog.asserts #1664

rachel-fenichel · 2018-07-24T23:25:47Z

Not urgent to review or merge this week.

Resolves

None

Proposed Changes

Mostly just pulling in these changes: RaspberryPiFoundation/blockly@3909bd4

Reason for Changes

Blockly is moving away from using closure library code where possible. We're starting with the easy cases/places where modern browsers standardized so there's no need to hide special casing behind function calls. The eventual goal is to remove the UI widgets and all other dependencies as well.

This change keeps scratch blocks in line with some of the recent blockly work. There will be other related changes.

Test Coverage

I built and played with the vertical playground.

core/block.js

 Blockly.Block.prototype.setFieldValue = function(newValue, name) {
  var field = this.getField(name);
-  goog.asserts.assertObject(field, 'Field "%s" not found.', name);
+  if (!field) {


core/block.js

    if (!this.previousConnection) {
-      goog.asserts.assert(!this.outputConnection,
-          'Remove output connection prior to adding previous connection.');
+      if (this.outputConnection) {


core/block.js

 Blockly.Block.prototype.mixin = function(mixinObj, opt_disableCheck) {
-  if (goog.isDef(opt_disableCheck) && !goog.isBoolean(opt_disableCheck)) {
-    throw new Error("opt_disableCheck must be a boolean if provided");
+  if (opt_disableCheck !== undefined && typeof opt_disableCheck != 'boolean') {


core/block_drag_surface.js

 Blockly.BlockDragSurfaceSvg.prototype.setBlocksAndShow = function(blocks) {
-  goog.asserts.assert(
-      this.dragGroup_.childNodes.length == 0, 'Already dragging a block.');
+  if (this.dragGroup_.childNodes.length) {


core/block_drag_surface.js

  this.SVG_.style.display = 'none';
-  goog.asserts.assert(
-      this.dragGroup_.childNodes.length == 0, 'Drag group was not cleared.');
+  if (this.dragGroup_.childNodes.length) {


core/extensions.js

 */
 Blockly.Extensions.registerMixin = function(name, mixinObj) {
-  if (!goog.isObject(mixinObj)){
+  if (!mixinObj || typeof mixinObj != 'object'){


core/field.js

    throw new Error('Invalid field type "' + type + '"');
  }
-  if (!goog.isObject(fieldClass) || !goog.isFunction(fieldClass.fromJson)) {
+  if (!fieldClass || (typeof fieldClass.fromJson != 'function')) {


core/utils.js

+ */
+Blockly.utils.startsWith = function(str, prefix) {
+  return str.lastIndexOf(prefix, 0) == 0;
+};


core/workspace_drag_surface_svg.js

  this.SVG_.style.display = 'none';
-  goog.asserts.assert(
-      this.SVG_.childNodes.length == 0, 'Drag surface was not cleared.');
+  if (this.SVG_.childNodes.length) {


core/workspace_drag_surface_svg.js

    blockCanvas, bubbleCanvas, previousSibling, width, height, scale) {
-  goog.asserts.assert(
-      this.SVG_.childNodes.length == 0, 'Already dragging a block.');
+  if (this.SVG_.childNodes.length) {


rachel-fenichel · 2018-08-02T18:52:22Z

@NeilFraser can you comment on some of the RHS/LHS questions? This PR is all code that you changed in Blockly.

kchadha · 2018-08-03T17:21:35Z

@NeilFraser Thanks for answering all the questions!

@rachel-fenichel This change looks good to go, but I think we are holding off on making big changes while the new 3.0 banner is up on the scratch website.

cc/ @thisandagain

rachel-fenichel · 2018-08-14T21:26:19Z

Friendly ping--can we merge this yet?

kchadha · 2018-08-16T13:44:44Z

@rachel-fenichel, I just talked to Ray, and he said it should be okay to merge.

kchadha

LG

Throw Errors instead of using goog.asserts

c88b1f8

rachel-fenichel requested a review from kchadha July 24, 2018 23:25

thisandagain added the pr - needs review label Jul 31, 2018

thisandagain assigned kchadha Jul 31, 2018

thisandagain added this to the August 2018 milestone Jul 31, 2018