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

EZP-25713: Reduce size of asset bundle further #13

Merged
merged 1 commit into from May 12, 2016

Conversation

andrerom
Copy link
Contributor

@andrerom andrerom commented Apr 21, 2016

https://jira.ez.no/browse/EZP-25713

YUI:

  • $YUI3_DIR/build//-debug.js
  • $YUI3_DIR/build//-coverage.js

Build folder total is 36,7mb, and these files are the biggest once.
_yui

Alloy:

  • $ALLOY_DIR/api
  • $ALLOY_DIR/lib

This assumes dist/ folder is all we need in prod.
_alloy_all

For future tuning we can look into if parts of dist/ folder can be removed as well, ref:

_alloy

@bdunogier
Copy link
Member

So what is the new size, and what was the old one ? In any case, it looks good. Tiny +1.

@andrerom
Copy link
Contributor Author

70M => 31M

@bdunogier
Copy link
Member

70M => 31M

You're quite the lumberjack. Bigger +1 then :-)

@andrerom
Copy link
Contributor Author

andrerom commented Apr 21, 2016

😄 Well remains to be seen what @dpobel thinks, he knows better what these files are for, and also if we can reduce 3-4mb more by removing unused stuff in alloy-editor/dist.

@bdunogier
Copy link
Member

All good.

Let's merge this to begin with. If we could get a smaller version of assets for the 1.3 release, it would be really cool.

@andrerom
Copy link
Contributor Author

Sure, but I'm not fully aware what these files removed here do, so would still hold back for UI team review.

@yannickroger
Copy link
Contributor

We should create an issue so we could highlight this in the release note. After all, this is a nice improvement of the release people should know about it 😄

@andrerom andrerom changed the title Reduce size of asset bundle further EZP-25713: Reduce size of asset bundle further Apr 22, 2016
@andrerom
Copy link
Contributor Author

Added issue

@dpobel
Copy link
Contributor

dpobel commented Apr 22, 2016

I'm ok with the change in YUI3 but I'm not on the one for AlloyEditor especially because this will break the PlatformUI Grunt task alloy-css. AlloyEditor's API doc can be removed though.

@andrerom
Copy link
Contributor Author

I'm not on the one for AlloyEditor especially

which one? there is 3 besides api/.

@dpobel
Copy link
Contributor

dpobel commented Apr 25, 2016

@andrerom I don't really know to be honest, you'll have to test which files are needed for grunt alloy-css (defined in https://github.com/ezsystems/PlatformUIBundle/blob/master/Gruntfile.js#L244)

@andrerom
Copy link
Contributor Author

Had a look, seems at least src/ is needed, ref alloySkinBasePath = alloyBase + "src/ui/react/src/assets/sass/skin/",

@andrerom
Copy link
Contributor Author

andrerom commented May 10, 2016

@dpobel Could you please have a look on this PR? Context: Travis Behat tests fails now on php 5.x as the asset package makes it tip over amount of memory allowed during composer install. I'll increase the 1Gb limit, but If we have this issue, others have as well..

YUI:
- $YUI3_DIR/build/*/*-debug.js
- $YUI3_DIR/build/*/*-coverage.js

Alloy:
- $ALLOY_DIR/api
- $ALLOY_DIR/lib
@andrerom
Copy link
Contributor Author

andrerom commented May 11, 2016

Updated PR, summary:

  • re added alloy test folder, needed for skins, ref Error: Cannot find module '../test/core/_src.js'
  • verified that grunt alloy-css works with the following patch to alloy: liferay/alloy-editor@1fb4fcd

So this is good to go, but we will also need to update alloy to 1.x t0 be able to generate skins again, as bourbon lib from master is currently broken.

@dpobel
Copy link
Contributor

dpobel commented May 11, 2016

+1

@andrerom
Copy link
Contributor Author

ping @yannickroger @StephaneDiot

@StephaneDiot
Copy link

+1 FWIW

@andrerom
Copy link
Contributor Author

andrerom commented May 12, 2016

+1 FWIW

It's worth it's weight in gold. In other words: thanks, merging :)

@andrerom andrerom merged commit 93f8de4 into master May 12, 2016
@andrerom andrerom deleted the reduce_package_size branch May 12, 2016 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants