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

refactor(build)!: Provide all generator exports when loaded as script #7169

Merged
merged 1 commit into from
Jun 15, 2023

Conversation

cpcallen
Copy link
Contributor

@cpcallen cpcallen commented Jun 15, 2023

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

Implements proposal 3 (option 3B) of #7086.

Reason for Changes

Previously, when loading a generator chunk (e.g., javascript_compressed.js) as a script (e.g., in a browser using a <SCRIPT> tag), only a single named export from that chunk would be made available (e.g, javascriptGenerator would be made available as Blockly.JavaScript).

Until recently, that was fine because each generator chunk had only a single named export, but now each one additionally has a <Lang>Generator class and Order enum export. At present developers loading generator chunks as scripts have no way to access these additional named exports.

Proposed Changes

To allow these new exports to be accessed by script users, the chunk wrappers are modified to provide the whole export object at a correspondingly-named global variable—e.g., when loaded as a script, javascript_compressed.js creates a global variable named javascript, so the named exports can be accessed as javascript.javascriptGenerator, javascript.JavascriptGenerator and javascript.Order, as if the user had imported them via

import * as javascript from 'blockly/javascript';

Behaviour Before Change

javascript_compressed.js's chunk wrapper looks like:

;(function(root, factory) {
  if (typeof define === 'function' && define.amd) { // AMD
    define(["./blockly_compressed.js"], factory);
  } else if (typeof exports === 'object') { // Node.js
    module.exports = factory(require("./blockly_compressed.js"));
  } else { // Browser
    var factoryExports = factory(root.Blockly);
    root.Blockly.JavaScript = factoryExports.javascriptGenerator;
    root.Blockly.JavaScript.__namespace__ = factoryExports.__namespace__;
  }
}(this, function(__parent__) {
var $=__parent__.__namespace__;
// COMPRESSED CODE OMITTED
module$build$src$generators$javascript$all.__namespace__=$;
return module$build$src$generators$javascript$all;
}));

Note that this sets only Blockly.Javascript.

Behaviour After Change

javascript_compressed.js's chunk wrapper looks like:

;(function(root, factory) {
  if (typeof define === 'function' && define.amd) { // AMD
    define(["./blockly_compressed.js"], factory);
  } else if (typeof exports === 'object') { // Node.js
    module.exports = factory(require("./blockly_compressed.js"));
  } else { // Script
    root.javascript = factory(root.Blockly);
    root.Blockly.Javascript = root.javascript.javascriptGenerator;
  }
}(this, function(__parent__) {
var $=__parent__.__namespace__;
// COMPRESSED CODE OMITTED
module$build$src$generators$javascript$all.__namespace__=$;
return module$build$src$generators$javascript$all;
}));

Note that this sets both javascript and Blockly.Javascript.

Test Coverage

Passes npm test; will need to verify that generators are fully working when loaded via <SCRIPT> tags (but this will happen automatically when we update the test server, which runs in compressed mode when served from a non-local web server).

Documentation

We should update any generator usage documentation and codelabs that use <SCRIPT> tags to use the new export objects.

Additional Information

BREAKING CHANGE:: The generator chunks will, when loaded as scripts (e.g. via a <SCRIPT> tag, now clobber any existing global variable of the corresponding name:

  • dart_compresed.js will set dart
  • javascript_compresed.js will set javascript
  • lua_compresed.js will set lua
  • php_compresed.js will set php
  • python_compresed.js will set python

DEPRECATION: Accessing the generator instances at their previous locations (Blockly.Dart, Blockly.JavaScript, Blockly.Lua, Blockly.PHP, and Blockly.Python) is deprecated and may cease to work in a future version of Blockly.

@cpcallen cpcallen requested a review from a team as a code owner June 15, 2023 00:23
@cpcallen cpcallen requested a review from BeksOmega June 15, 2023 00:23
@cpcallen cpcallen added the deprecation This PR deprecates an API. label Jun 15, 2023
@github-actions github-actions bot added PR: refactor and removed deprecation This PR deprecates an API. labels Jun 15, 2023
@cpcallen cpcallen added component: generators deprecation This PR deprecates an API. breaking change Used to mark a PR or issue that changes our public APIs. labels Jun 15, 2023
@cpcallen cpcallen force-pushed the refactor/7086/script-exports branch from 0d53c49 to 0abdb9e Compare June 15, 2023 13:31
@github-actions github-actions bot added PR: refactor and removed breaking change Used to mark a PR or issue that changes our public APIs. PR: refactor deprecation This PR deprecates an API. labels Jun 15, 2023
Previously, when loading a generator chunk (e.g.,
javascript_compressed.js) as a script (e.g., in a browser using
a <SCRIPT> tag), only a single named export from that chunk would
be made available (e.g, javascriptGenerator would be made availabe
as Blockly.JavaScript).

Until recently, that was fine because each generator chunk had only
a single named export, but now each one additionally has a
<Lang>Generator class and Order enum export.

To allow these new exports to be accessed by script users, the
chunk wrappers are modified to provide the whole export object
at a correspondingly-named global variable—e.g., when loaded as
a script javascript_compressed.js creates a global variable named
javascript, so the named exports can be accessed as
javascript.javascriptGenerator, javascript.JavascriptGenerator
and javascript.Order, as if the user had imported them via

    import * as javascript from 'blockly/javascript';

This PR includes a breaking change and a deprecation, both of
which are only applicable when loading generators as scripts
(e.g. via a <SCRIPT> tag):

BREAKING CHANGE: The generator chunks will, when loaded as scripts
(e.g. via a <SCRIPT> tag, now clobber any existing global variable
of the corresponding name:

- dart_compresed.js will set dart
- javascript_compresed.js will set javascript
- lua_compresed.js will set lua
- php_compresed.js will set php
- python_compresed.js will set python

DEPRECATION: Accessing the generator instances at their previous
locations (Blockly.Dart, Blockly.JavaScript, Blockly.Lua,
Blockly.PHP, and Blockly.Python) is deprecated and may cease
to work in a future version of Blockly.
@cpcallen cpcallen force-pushed the refactor/7086/script-exports branch from 0abdb9e to 5984070 Compare June 15, 2023 13:33
@cpcallen
Copy link
Contributor Author

Updated PR description and force-pushed to update commit message to note that there is in fact a breaking change.

@cpcallen cpcallen changed the title refactor(build): Provide all generator exports when loaded as script refactor(build)!: Provide all generator exports when loaded as script Jun 15, 2023
@github-actions github-actions bot added breaking change Used to mark a PR or issue that changes our public APIs. PR: refactor and removed PR: refactor labels Jun 15, 2023
@cpcallen cpcallen merged commit 2d97e5a into google:develop Jun 15, 2023
@cpcallen cpcallen deleted the refactor/7086/script-exports branch June 15, 2023 15:52
@cpcallen cpcallen added the deprecation This PR deprecates an API. label Jun 15, 2023
@maribethb
Copy link
Contributor

maribethb commented Jun 22, 2023

@cpcallen This PR sets the script export to Blockly.Javascript but I believe the old version was Blockly.JavaScript, so this would need to be changed in order to be backwards-compatible. Can you confirm?

Also, only JavaScript and Python set the old backwards-compatible Blockly.lang object in their UMD wrappers. Lua, Dart, and PHP only set their new global identifier. I am not sure why that would be the case based on this PR but assume it has something to do with the fact that JS and Python are the two included in the main blockly module.

In other words, while it was my understanding that
Blockly.JavaScript, Blockly.Python, Blockly.Lua, Blockly.PHP, and Blockly.Dart should continue to work for now (although deprecated), it appears that only Blockly.Python still works. So we need to address this before we release.

reexport: 'Blockly.JavaScript',
reexportOnly: 'javascriptGenerator',
scriptExport: 'javascript',
scriptNamedExports: {'Blockly.Javascript': 'javascriptGenerator'},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @maribethb noted: this should have been Blockly.JavaScript, not …Javascript.

@cpcallen
Copy link
Contributor Author

cpcallen commented Jun 22, 2023

Also, only JavaScript and Python set the old backwards-compatible Blockly.lang object in their UMD wrappers. Lua, Dart, and PHP only set their new global identifier.

That is unexpected.

I am not sure why that would be the case based on this PR but assume it has something to do with the fact that JS and Python are the two included in the main blockly module.

That doesn't make any sense (because the "main blockly module" isn't normally used for script imports)—but is probably a very useful clue.

[Edited to add: also doesn't make any sense because the "main blockly module" is produced by a completely separate set of gulp tasks, using mostly different input files.]

In other words, while it was my understanding that Blockly.JavaScript, Blockly.Python, Blockly.Lua, Blockly.PHP, and Blockly.Dart should continue to work for now (although deprecated), it appears that only Blockly.Python still works. So we need to address this before we release.

Yes, those should all work (provided you <script src='path/to/lang_compressed.js'>, of course).

@cpcallen
Copy link
Contributor Author

Also, only JavaScript and Python set the old backwards-compatible Blockly.lang object in their UMD wrappers.

No, PHP does as well, it turns out.

cpcallen added a commit to cpcallen/blockly that referenced this pull request Jun 22, 2023
Three separate mistakes left only the Python and PHP chunks with
the correct code for the legacy script exports.
@maribethb
Copy link
Contributor

No, PHP does as well, it turns out.

GitHub needs to add the 🤔 emoji as a reaction.

cpcallen added a commit that referenced this pull request Jun 22, 2023
Three separate mistakes left only the Python and PHP chunks with
the correct code for the legacy script exports.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Used to mark a PR or issue that changes our public APIs. component: generators deprecation This PR deprecates an API. PR: refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: Library generator publication changes + supporting change to CodeGenerator
3 participants