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

Remove closure base file dependency #2976

Merged
merged 20 commits into from
Sep 12, 2019

Conversation

samelhusseini
Copy link
Contributor

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

The details

Resolves

Shave off 28KB from Blockly compressed.

Proposed Changes

Because we made use of a number of closure's base methods, we had a need to include closure's base file. Since we only use very few of these methods, we can trim down our binary size even further by providing alternatives for the following:

  • goog.inherits
  • goog.exportSymbol
  • goog.mixin
  • goog.require
  • goog.provide

goog.exportSymbol and goog.mixin can be moved to Blockly.utils and re-implemented.

goog.provide and goog.require are built in and the compiler knows how to handle those. We do need to stub those though, because our message files include a require and provide call.

goog.inherits I've toyed with a few ideas of what to do with this one, but the least intrusive solution I came up with was to just inject an implementation of it.

@NeilFraser thoughts?

Reason for Changes

Less closure (save 28KB)

Test Coverage

Tested the demos.

core/utils.js Outdated Show resolved Hide resolved
core/utils.js Outdated Show resolved Hide resolved
build.py Outdated Show resolved Hide resolved
build.py Outdated Show resolved Hide resolved
Copy link
Contributor

@NeilFraser NeilFraser left a comment

Choose a reason for hiding this comment

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

I still want to poke this deeper, but here's some self-contained items.

core/utils.js Outdated Show resolved Hide resolved
core/workspace.js Outdated Show resolved Hide resolved
demos/custom-fields/pitch/field_pitch.js Outdated Show resolved Hide resolved
gulpfile.js Outdated Show resolved Hide resolved
build.py Outdated Show resolved Hide resolved
@samelhusseini
Copy link
Contributor Author

@NeilFraser this is ready for a final review.

tests/compile/main.js Outdated Show resolved Hide resolved
@samelhusseini samelhusseini merged commit 8ab51c8 into google:develop Sep 12, 2019
@samelhusseini samelhusseini deleted the evenlessclosure branch September 12, 2019 00:30
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