Refactor BlockDefinition to support blocks defined in JavaScript.

[AnmAtAnm]

Adding .javascript_ field to support block definitions defined in JavaScript. Adding a format string parameter (either 'JSON' or 'JS) to many of the methods.

Next step: Updates to JSON definition are still not updating the preview. Check BlockEditorController.updatePreview() or .refreshPreview().

---

This change is 

[rachel-fenichel]

Review status: 0 of 5 files reviewed at latest revision, 3 unresolved discussions.

---

src/model/block_definition.js, line 57 at r1 (raw file):

    /**
     * The JSON representation of the block.

Fix comment.

---

src/model/block_definition.js, line 121 at r1 (raw file):

   */
  defineFromJson_() {
    const typeName = this.type();

You removed the check that the name exists/is valid. Why?

---

src/model/block_definition.js, line 154 at r1 (raw file):

      console.error('Error while evaluating JavaScript formatted block definition', e);
      undefine();  // Attempt to reset state.
      throw new Error('Failed to define block type from JavaScript.');

Why do you have both a console.error and a new Error?

---

Comments from Reviewable

[picklesrus]

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions.

---

src/model/block_definition.js, line 34 at r1 (raw file):

   * BlockDefinition Class.
   * @constructor
   * @param {string} blockTypeName The type name of the definition.

What is a TypeName in this context? I thought it was going to be json or js but that's in format.

---

Comments from Reviewable

[AnmAtAnm]

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions.

---

src/model/block_definition.js, line 34 at r1 (raw file):

Previously, picklesrus wrote…

What is a TypeName in this context? I thought it was going to be json or js but that's in format.

This is the name of the block type, the one used as a key to Blockly.Blocks[] or the 'name' field of JSON.

---

src/model/block_definition.js, line 57 at r1 (raw file):

Previously, rachel-fenichel (Rachel Fenichel) wrote…

Fix comment.

Done.

---

src/model/block_definition.js, line 121 at r1 (raw file):

Previously, rachel-fenichel (Rachel Fenichel) wrote…

You removed the check that the name exists/is valid. Why?

Added back.

---

src/model/block_definition.js, line 154 at r1 (raw file):

Previously, rachel-fenichel (Rachel Fenichel) wrote…

Why do you have both a console.error and a new Error?

If I make make a guess about my former self... There is no 'cause' field in JavaScript errors, so logging the original error to console.error(..) captures the deeper stack trace. Throwing a different error lets me give a description that may make more sense in the calling context.

Added a comment above console.error(..).

---

Comments from Reviewable

[rachel-fenichel]

---

Review status: 0 of 5 files reviewed at latest revision, 4 unresolved discussions.

---

Comments from Reviewable