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

fix(generators): Changes to exports and access controls for TypeScript compatibility #7295

Merged
merged 4 commits into from
Jul 17, 2023

Conversation

cpcallen
Copy link
Contributor

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 #7283.
Also partially fixes google/blockly-samples#1785.

Proposed Changes

  • Add missing declarations for Order enums to typings/<language>.d.ts.

    This is a temporary measure until the generators are converted to TypeScript and the .d.ts files can be autogenerated.)

  • Remove the protected declaration on the provideFunction_ method and FUNCTION_NAME_PLACEHOLDER_ property on CodeGenerator so they can be used from generator functions written in TypeScript.

    Previously generator functions were (ES5-style) methods on (Code)Generator instances, and Closure Compiler was apparently happy with this, but now these functions live in a separate dictionary and tsc is pickier.

Behaviour Before Change

It was not possible to write code generators in TypeScript that made use of Order, provideFunction_ or FUNCTION_NAME_PLACEHOLDER_. The former limitation had to be worked around by using the now-deprecated ORDER_* properties on the CodeGenerator instance instead.

Behaviour After Change

It is now possible to use all three features in TypeScript.

Reason for Changes

Fix bugs.

Test Coverage

Manually tested changes (to javascript/javascript_generator.js only) via blockly-samples' examples/sample-app-ts.

Documentation

We should regenerate the documentation for CodeGenerator to remove the protected declaration.

It turns out we have no auto-generated reference documentation for anything in generators/ so there is nothing needing updating on that front. 🙁

Additional Information

This fix should be cherry-picked into a patch release ASAP.

cpcallen added 3 commits July 14, 2023 18:52
Remove the protected declaration on provideFunction_ and
FUNCITON_NAME_PLACEHOLDER_ so they can be used from generator
functions written in TypeScript.

Not strictly part of google#7283, but closely related and required to
fixing the related issue google/blockly-samples#1785.
@cpcallen cpcallen requested a review from a team as a code owner July 17, 2023 16:45
@cpcallen cpcallen requested a review from rachel-fenichel July 17, 2023 16:45
@github-actions github-actions bot added the PR: fix Fixes a bug label Jul 17, 2023
@cpcallen cpcallen requested review from BeksOmega and removed request for rachel-fenichel July 17, 2023 17:08
Copy link
Collaborator

@BeksOmega BeksOmega left a comment

Choose a reason for hiding this comment

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

LGTM!

@cpcallen cpcallen merged commit d503fbb into google:develop Jul 17, 2023
@cpcallen cpcallen deleted the fix/7283 branch July 17, 2023 19:40
ericblackmonGoogle pushed a commit that referenced this pull request Jul 17, 2023
…t compatibility (#7295)

* fix(generators): Add missing declarations for Order enums

* chore(generators): Remove spurious whitespace

* fix(generators): Make provideFunction_ etc. public

  Remove the protected declaration on provideFunction_ and
  FUNCTION_NAME_PLACEHOLDER_ so they can be used from generator
  functions written in TypeScript.

  Not strictly part of #7283, but closely related and required to
  fixing the related issue google/blockly-samples#1785.

* chore(generators): format

(cherry picked from commit d503fbb)
ericblackmonGoogle added a commit that referenced this pull request Jul 17, 2023
* fix(generators): Changes to exports and access controls for TypeScript compatibility (#7295)

* fix(generators): Add missing declarations for Order enums

* chore(generators): Remove spurious whitespace

* fix(generators): Make provideFunction_ etc. public

  Remove the protected declaration on provideFunction_ and
  FUNCTION_NAME_PLACEHOLDER_ so they can be used from generator
  functions written in TypeScript.

  Not strictly part of #7283, but closely related and required to
  fixing the related issue google/blockly-samples#1785.

* chore(generators): format

(cherry picked from commit d503fbb)

* fix: Correct errors in `HSV_SATURATION`, `HSV_VALUE` accessors (#7297)

* fix: Correct errors in HSV_SATURATION, HSV_VALUE accessors

  Fix the comment / message errors noted in
  #7249 (comment)

* chore: Add renamings for HSV_SATURATION, HSV_VALUE

(cherry picked from commit 1bc4f67)

* release: Update version number to 10.0.2

---------

Co-authored-by: Christopher Allen <[email protected]>
ericblackmonGoogle added a commit that referenced this pull request Jul 17, 2023
* fix(generators): Changes to exports and access controls for TypeScript compatibility (#7295)

* fix(generators): Add missing declarations for Order enums

* chore(generators): Remove spurious whitespace

* fix(generators): Make provideFunction_ etc. public

  Remove the protected declaration on provideFunction_ and
  FUNCTION_NAME_PLACEHOLDER_ so they can be used from generator
  functions written in TypeScript.

  Not strictly part of #7283, but closely related and required to
  fixing the related issue google/blockly-samples#1785.

* chore(generators): format

(cherry picked from commit d503fbb)

* fix: Correct errors in `HSV_SATURATION`, `HSV_VALUE` accessors (#7297)

* fix: Correct errors in HSV_SATURATION, HSV_VALUE accessors

  Fix the comment / message errors noted in
  #7249 (comment)

* chore: Add renamings for HSV_SATURATION, HSV_VALUE

(cherry picked from commit 1bc4f67)

* release: Update version number to 10.0.2

---------

Co-authored-by: Christopher Allen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sample-app-ts fails to build TypeScript unhappy with 'blockly/javascript' exports
3 participants