Skip to content

Conversation

@moniika
Copy link
Contributor

@moniika moniika commented Aug 19, 2021

The basics

  • I branched from goog_module
  • My pull request is against goog_module
  • My code follows the style guide
  • My code is presented in the form suggested in the module
    conversion guide
  • I have run npm run test locally already.

The details

Resolves

Part of #5026

Proposed Changes

Converts core/utils/useragent.js to goog.module syntax.

@moniika moniika requested a review from a team as a code owner August 19, 2021 16:59
@moniika moniika requested a review from maribethb August 19, 2021 16:59
@google-cla google-cla bot added the cla: yes Used by Google's CLA checker. label Aug 19, 2021
@github-actions github-actions bot added this to the 2021_q3_release milestone Aug 19, 2021
Blockly.utils.userAgent.IE;
/** @type {boolean} */
let IE;
exports.IE = IE;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this approach work? It seems like these are all going to be exported as undefined since their values aren't set until later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah: that is not going to work. For a bunch of exported booleans that are only referenced (in this module) for the purposes of setting other exported booleans, I'd suggest just setting them all directly on exports and not bothering with declaring local variables for them at all.

@cpcallen
Copy link
Collaborator

Oh dear. I'd forgotten that you were working on this, and inadvertently duplicated this work in #5435. My apologies.

I'm going to close this, but feel free to reopen and/or submit a new PR correcting any problems with my PR.

@cpcallen cpcallen closed this Sep 10, 2021
@moniika moniika deleted the convert-useragent branch September 24, 2021 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes Used by Google's CLA checker. type: cleanup

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants