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!: remove backwards compatible get and set accessors #6203

Closed
wants to merge 2 commits into from

Conversation

BeksOmega
Copy link
Collaborator

@BeksOmega BeksOmega commented Jun 9, 2022

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

N/A

Proposed Changes

Removes some get and set accessors (most of which were deprecated).

  • Blockly.alert
  • Blockly.prompt
  • Blockly.confirm
  • Blockly.mainWorkspace
  • Blockly.selected
  • Blockly.HSV_SATURATION
  • Blockly.HSV_VALUE
  • Blockly.ContextMenu.currentBlock
  • Blockly.Events.recordUndo
  • Blockly.Tooltip.visible
  • Blockly.Tooltip.DIV
  • Blockly.WidgetDiv.DIV

Behavior Before Change

These accessors were available.

Behavior After Change

These accessor are not available.

Reason for Changes

As we move to typescript, it's no longer possible to provide accessors directly on the exports object, so these have to be removed.

Test Coverage

N/A

How to Migrate

You can run the blockly/migrate script to automatically perform renamings in your files. If you are setting any of these values, you may need to do some manual edits to properly call the setter function.

@BeksOmega BeksOmega requested a review from a team as a code owner June 9, 2022 21:39
@BeksOmega BeksOmega requested a review from cpcallen June 9, 2022 21:39
@BeksOmega BeksOmega force-pushed the fix/remove-accessors branch from 5028af2 to 983f028 Compare June 9, 2022 21:44
Copy link
Contributor

@cpcallen cpcallen left a comment

Choose a reason for hiding this comment

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

This might be a good subject for a team discussion, but I lean towards leaving the ones in blockly.js alone until we are actually ready to convert that file, which we may find we do not want to do until a subsequent release as it entails some (drafted but not finished) changes to the UMD wrappers.

Actually, this brings up another point: we could monkey-patch in these accessors onto the malleable compiled ES5 code (doing this in one of the NPM package wrappers), allowing them to continue to work even though this would not be possible in uncompiled mode.

@BeksOmega
Copy link
Collaborator Author

I think you're the only one with enough context to be able to write up alternatives and add it to the meeting @cpcallen

@cpcallen
Copy link
Contributor

cpcallen commented Jul 5, 2022

Closing this as obsolete. It was helpful, though, to make sure I didn't miss anything in #6260.

@cpcallen cpcallen closed this Jul 5, 2022
@BeksOmega BeksOmega deleted the fix/remove-accessors branch October 4, 2022 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants