fix: Make message files work in unpackaged mode, and rebuild msg files

[maribethb]

The basics

• I branched from develop
• My pull request is against develop
• My code follows the style guide
• I ran npm run format and npm run lint

The details

Resolves

Fixes #6123

Proposed Changes

Updates the Msg files to use Blockly.Msg instead of messages, and associated changes in the preamble and wrapper to make that approach work in all cases.

n.b. Thanks to @cpcallen for suggesting the approach taken here.

Behavior Before Change

• Message loading works when importing as a module
• Message loading works when loading the wrapped message files through a script tag
• Message loading does not work when loading the unwrapped message files through a script tag

Behavior After Change

• Message loading works when importing as a module
• Message loading works when loading the wrapped message files through a script tag
• Message loading works when loading the unwrapped message files through a script tag

Reason for Changes

In #6184 the message files were changed. Instead of assigning each translation directly on Blockly.Msg, the translations were put on a new object called messages. In the UMD wrapper for the language files, all of the properties of that new object are then placed on Blockly.Msg. This works for both of the first two cases above, but it doesn't work for anyone who is using the unwrapped message files.

That is a scenario we need to support for now since it is explicitly in our documentation that you can download and run the code from github.

Also, the file blockly.min.js in the packaged output does not use the wrapped message file, so e.g. using a script tag to load <script src="https://unpkg.com/blockly"></script> does not work because the unwrapped message files don't load prior to this change. That line of code is in the Getting Started codelab so that also needs to work. That will be fixed in a follow up PR.

Test Coverage

• Playground in core loading msg/messages.js (default behavior. this is the raw message file that isn't run through the python script and does not have the umd wrapper so it is not a good test of any real world behavior and should probably be changed)
• Playground in core loading 'dist/msg/es.js' instead (compiled through python script and umd wrapped)
• Playground in core loading 'msg/js/es.js' instead (compiled through python script but not umd wrapped)
• scroll-options plugin linking to this build of blockly (verified through inspecting node_modules that it points to this build and not a random other plugin's old version of blockly...)
• scroll options plugin with these changes in test/index.js:

import * as Fr from 'blockly/msg/fr';
import * as Es from 'blockly/msg/es';
// ...
function createWorkspace(blocklyDiv, options) {
  Blockly.setLocale(Fr);
// ...
}

• and with these changes:

import * as Fr from 'blockly/msg/fr';
import * as Es from 'blockly/msg/es';
// ...
function createWorkspace(blocklyDiv, options) {
  Blockly.setLocale(Fr);
  Blockly.setLocale(Es);
// ...
}

• Getting Started codelab, replacing <script src="https://unpkg.com/blockly"></script> with <script src="../../../../blockly/dist/blockly.min.js"></script> (linked to this build of blockly. this is the equivalent file that gets loaded when visiting the unpkg link. it contains the unwrapped english message file as noted above.)
• Same as above but also adding <script src="../../../../blockly/dist/msg/fr.js"></script> (compiled and umd wrapped)
• Block factory demo in core using msg/js/en.js (default - compiled but not umd wrapped)
• Block factory demo in core using dist/msg/az.js (compiled and umd wrapped)
• Plane demo in samples, linked to this build of blockly

[BeksOmega]

I think this makes sense and covers all of our cases (given I thought our previous fixes did as well, but alas).

• In the browser, we want to overwrite both the Blockly tree, and the module-junk (as defined here).
  • This does that in the unwrapped case, by accessing the global Blockly and then assigning to it, which also modifies the module junk.
  • This does this in the wrapped case, by creating a new object {Msg: Object.create(null)} and then allowing the uml wrapper to use a for in loop to assign the messages to the Blockly tree which also modifies the module junk.
• In a module system, we do not want to overwrite either the Blockly tree or the module-junk directly. Instead we want to let Blockly.setLocale do that.
  • This does this by creating a new object {Msg: Object.create(null)} and then allowing the external developer to call setLocale with the returned value.

I think that technically (if my understanding is correct) you can actually use:

var messages = Blockly || {Msg: Object.create(null)};

and then use messages.Msg in the rest of the file. I kind of think this is clearer, because in the browser wrapped case, and in the module system case, you're not actually modifying Blockly.Msg directly (that is done in a separate step). But I guess in the unwrapped browser case you are modifying it directly... so up to you.

[cpcallen]

I think that technically (if my understanding is correct) you can actually use:

var messages = Blockly || {Msg: Object.create(null)};

and then use messages.Msg in the rest of the file. I kind of think this is clearer, because in the browser wrapped case, and in the module system case, you're not actually modifying Blockly.Msg directly (that is done in a separate step). But I guess in the unwrapped browser case you are modifying it directly... so up to you.

You are correct. However:

• It's safer not to introduce additional variables into the global scope in the case that these files are loaded unwrapped via a <script> tag (which shouldn't happen but evidently does), so var Blockly is preferable to var messages in that case.
• The object with the .Msg property is a stand-in for the Blockly object, not (now) a stand-in for the Blockly.Msg dictionary, so calling it Blockly is less at least no more confusing than calling it messages.

[cpcallen]

Thanks for doing all the careful testing, which is really the important part of this commit!

My only thought was to question whether msg/messages.js should get the same treatment, but:

• it already references Blockly.Msg (good), and
• doesn't get wrapped so (I think)
• can only be loaded as a <script> and so (I'm fairly sure)
• doesn't need the var Blockly = Blockly || ... preamble.

Oh, and minor note about presentation: I would have found this PR slightly nicer to review if the script changes had been done in one commit and then the generated output updated in a second. (As it is the interesting bit gets buried at the end of a lot of boilerplate changes.)

[maribethb]

Thanks for the review!

for msg/messages.js I don't think anyone should ever require this, script tag or no. This is the input to the language compilation script and it shouldn't be used directly (I think it's a mistake that our playground does and we should change that). But yes if someone does happen to use it then it should work in its current state.

Sorry about the giant commit, I realized after I had already pushed that I should have done it differently.

[cpcallen]

For msg/messages.js I don't think anyone should ever require this, script tag or no. This is the input to the language compilation script and it shouldn't be used directly (I think it's a mistake that our playground does and we should change that).

I concur.

Sorry about the giant commit, I realized after I had already pushed that I should have done it differently.

No problem. I was only ever going to spot-check the language files anyway, so it was just a matter of making sure that I had got right to the end.

[cpcallen]

While reviewing PR #7661 I discovered that I had overlooked a confusing feature of this line:

I read var Blockly = Blockly … to mean var Blockly = globalThis.Blockly …, and indeed when the generated files (e.g. build/msg/en.js) are loaded as scripts it does mean just that—but after being UMD wrapped (as e.g. dist/msg/en.js) the second Blockly ends up referring to the var Blockly being defined on the same line, and so is always undefined at this point (and that's true whether loaded as a CJS module or as a script).

Interestingly this does not cause any problems, because only playgrounds and tests load the unwrapped build/msg/en.js (and do so as a <script>), while the UMD wrapper added to (e.g.) dist/msg/en.js contains special code to copy the message properties one-by-one from the temporary object onto the actual Blockly.Msg dictionary—but only when the wrapped file is loaded as a script.