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: deprecate functions in object.ts #6387

Merged
merged 3 commits into from
Aug 30, 2022

Conversation

rachel-fenichel
Copy link
Collaborator

@rachel-fenichel rachel-fenichel commented Aug 26, 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

Part of #6325.

Proposed Changes

  • Removes special casing for IE in object.values.
  • Marks object.values and object.inherits as deprecated, with removal dates.
  • Switches to using Object.values directly in shortcut_registry.js.
  • Fixes exposed type errors.
  • Changes the deprecation warning function to say things will be deleted after a date or a version.

Behavior Before Change

No change.

Behavior After Change

No change

Reason for Changes

Remove workaround and deprecate unused code in preparation for removal.
Improve types!

Test Coverage

Tested by opening up the playground and trying to register shortcuts with modifiers that do and don't exist:

Functional cases:

// Create a serialized key from the primary key and any modifiers.
var ctrlW = Blockly.ShortcutRegistry.registry.createSerializedKey(
    Blockly.utils.KeyCodes.W, [Blockly.ShortcutRegistry.modifierKeys.Control]);

Same as above, but not using the enum.

// Create a serialized key from the primary key and any modifiers.
var ctrlW = Blockly.ShortcutRegistry.registry.createSerializedKey(
    Blockly.utils.KeyCodes.W, [17]);

Broken case:

// Create a serialized key from the primary key and any modifiers.
var ctrlW = Blockly.ShortcutRegistry.registry.createSerializedKey(
    Blockly.utils.KeyCodes.W, [6]);

Documentation

None.

Additional Information

Should I shorten the deprecation date? I've been using a year by default but we could make it a quarter or two instead.

After discussion we decided to use the next major version instead of a date.

If this deprecation breaks you: you can use Object.values instead of object.values, but may need to cast the result to any or fix any type issues this reveals. You can use standard inheritance in JS or TS instead of our implementation of object.inherits.

@rachel-fenichel rachel-fenichel requested a review from a team as a code owner August 26, 2022 23:17
@rachel-fenichel rachel-fenichel added type: cleanup component: developer apis component: keyboard_nav breaking change Used to mark a PR or issue that changes our public APIs. PR: fix Fixes a bug deprecation This PR deprecates an API. labels Aug 26, 2022
@rachel-fenichel rachel-fenichel removed the breaking change Used to mark a PR or issue that changes our public APIs. label Aug 26, 2022
@rachel-fenichel rachel-fenichel changed the title fix!: deprecate functions in object.ts fix: deprecate functions in object.ts Aug 26, 2022
@rachel-fenichel rachel-fenichel merged commit 9775b51 into google:develop Aug 30, 2022
@rachel-fenichel rachel-fenichel deleted the cleanup_object branch August 30, 2022 23:20
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.

2 participants