fix(core): fix #989, remove unuse code, use shorter name to reduce bundle size by JiaLiPassion · Pull Request #990 · angular/zone.js

JiaLiPassion · 2018-01-06T23:54:50Z

remove unused code
move getUserMedia and electron related code to standalone files.
use shorter variable and function name to reduce bundle size.
now the size of zone.min.js will be 38510 or 38955 bytes. (the size will change when I rebuild)
add file size check logic in travis build.
change func.apply to func.call for better performance.

now I set a limitation of zone.min.js to 40000 bytes.

@mhevery , please review the idea is ok or not. thank you!

…duce bundle size

mhevery · 2018-01-08T22:39:57Z

 */

+// issue #989, to reduce bundle size, use short name
+export const a = Object.getOwnPropertyDescriptor;


there is no need to have a single char letters here. You can have long names, they will get renamed using uglify. Single letters are hard on readability.

Thanks for pointing out this one, I have misunderstanding about uglfiyjs, I have changed the variable name to long names.

mhevery · 2018-01-08T22:42:41Z

+export const k = 'true';
+export const l = 'false';
+export const m = '__zone_symbol__';
+export const n = 'function';


i think typeof x == 'string is faster than typeof x == STRING_CONST. That is because in one case the jitter can create the check code which directly checks type to be string, in tho other case it has to generate a more complex code which could be actual type and than compare it to the constant. I would revert this.

Got it, I have reverted all typeof x === parts.

mhevery · 2018-01-08T22:43:24Z

    const ignoreProperties: IgnoreProperty[] = _global.__Zone_ignore_on_properties;
    // for browsers that we can patch the descriptor:  Chrome & Firefox
    if (isBrowser) {
+      const w: any = window;


local vars get renamed. Calling it w makes it hard to read. Could we call it window and that way we don't have to change the code bellow? I know window = window would not work, but something like that?

yes, you are right, I have changed the variable to internalWindow.

mhevery · 2018-01-08T22:49:27Z

-
  performanceMeasure('Zone', 'Zone');
+  const z: any = Zone.prototype;
+  z.w = z.wrap;


I would prefer that we don't pollute the Zone public API with these short names. I realized they are not part of .d.ts but they will show up in debugger.

How about this idea
Zone.current could be replaced with getZoneCurrent() which is declared in util file. The uglify will rename the getZoneCurrent to something short like x.

Zone.current.wrap(...) could be wrapWithCurrentZone(...)

and so on. What do you think?

Yes, this is a better solution, I created wrapWithCurrentZone and scheduleMacroTaskWithCurrentZone helper method.

mhevery · 2018-01-08T22:50:05Z

+  z.si = z.scheduleMicroTask;
+  z.se = z.scheduleEventTask;
+  z.ct = z.cancelTask;
+  (Zone as any).l = Zone.__load_patch;


same thing here, helpers in util.ts can hide the long name without loosing readability

alfaproject

I don't think this is good for the maintainability of the project, and most of the things you changed will minify/compress just fine and with better results.

alfaproject · 2018-01-08T22:42:41Z


  Object.create = <any>function(obj: any, proto: any) {
-    if (typeof proto === OBJECT && !Object.isFrozen(proto)) {
+    // o is 'object' string


yes, I have reverted this one, thank you!

alfaproject · 2018-01-08T22:47:03Z

 * @suppress {undefinedVars,globalThis,missingRequire}
 */

+// issue #989, to reduce bundle size, use short name


You shouldn't do the job of a compressor.
In fact, you are probably adding entropy.

P.S.: all of these extra imports/exports add up to browser parsing/interpreting time as well

Yes, you are right, I have misunderstanding about uglifyjs, I have modified the PR.

…MacroTask

IgorMinar · 2018-01-10T18:54:17Z

awesome! thanks!

googlebot added the cla: yes label Jan 6, 2018

JiaLiPassion changed the title ~~fix(core): fix #989, remove unuse code, use shorter name to reduce bundle size~~ WIP(core): fix #989, remove unuse code, use shorter name to reduce bundle size Jan 7, 2018

fix(core): fix angular#989, remove unuse code, use shorter name to re…

11213c6

…duce bundle size

JiaLiPassion force-pushed the size branch from 61125af to 11213c6 Compare January 7, 2018 07:37

fix(core): fix shorter name closure conflict

0bb9664

JiaLiPassion force-pushed the size branch from df7c770 to 0bb9664 Compare January 7, 2018 10:54

fix(core): add comment for shorter var/function name

bbb7a21

JiaLiPassion changed the title ~~WIP(core): fix #989, remove unuse code, use shorter name to reduce bundle size~~ fix(core): fix #989, remove unuse code, use shorter name to reduce bundle size Jan 7, 2018

JiaLiPassion added 2 commits January 8, 2018 11:36

fix(core): add file check script in travis build

5928f9e

fix(core): add rxjs test

3e445e4

JiaLiPassion mentioned this pull request Jan 8, 2018

build(aio): upgrade to [email protected] angular/angular#21349

Closed

mhevery suggested changes Jan 8, 2018

View reviewed changes

alfaproject suggested changes Jan 8, 2018

View reviewed changes

JiaLiPassion force-pushed the size branch 2 times, most recently from e6eb346 to d7ef237 Compare January 9, 2018 06:37

fix(core): remove unreadable short names

9c30aaa

JiaLiPassion force-pushed the size branch from d7ef237 to 9c30aaa Compare January 9, 2018 06:47

fix(core): add helper method in util.ts to shorter zone.wrap/scehdule…

48ebd3e

…MacroTask

mhevery approved these changes Jan 10, 2018

View reviewed changes

mhevery merged commit 8293c37 into angular:master Jan 10, 2018

Conversation

JiaLiPassion commented Jan 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JiaLiPassion Jan 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alfaproject left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

IgorMinar commented Jan 10, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JiaLiPassion commented Jan 6, 2018 •

edited

Loading

JiaLiPassion Jan 9, 2018 •

edited

Loading