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

V8: Inline AngularJS templates into directives and controllers #4845

Closed
PeteDuncanson opened this issue Mar 4, 2019 · 9 comments

Comments

@PeteDuncanson
Copy link
Contributor

commented Mar 4, 2019

Currently all the templates for the various directives in the back office are pulled in via a templateUrl property. This causes angularjs to have to make a network request per template and load them all up to render the back office. There are a number of ways we can reduce that network traffic. I think we can do it purely via the existing build tools (Gulp) as well.

We can make some sizable performance improvements by changing the way we do this using one of two methods:

  1. Bundle all the templates into a single file and then pre-load them into the templateCache for quick access. Achievable via https://www.npmjs.com/package/gulp-inline-angular-templates and https://www.npmjs.com/package/gulp-angular-templatecache
  2. (preferred) Embed the templates as strings within the directives. Achievable via https://www.npmjs.com/package/gulp-angular-embed-templates

I prefer option 2 as it would give us the option of loading directives/components on the fly in the future which is something I'd love to move towards for yet more speed gains.

However either option comes at a bit of a cost regarding developers ability to poke around the code or to quickly make changes to the back office. You can't as easily hack around an AngularJS template for development once we go this route. Admittedly you shouldn't be doing this anyway as its easy to lose the change when you upgrade etc. but for quick and cheap "learning by hacking" we would lose some of the ease with which you could do that. To be fair I think this is totally worth it as in my eyes we should be moving to more cleaner code base which would enable this anyway and this is a step in the right direction, one of many I'd like to see it take on.

What can I do?

I'm happy to do a play around and see if I can get this in and working and then document any improvements in performance it makes and then update any docs on the tool chain that people should be aware of. Basically I'd love to just get it working and a PR put in :)

@PeteDuncanson

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2019

A rather excellent blog post on the speed benefits of doing the above is here https://medium.com/@frosty/angularjs-template-vs-templateurl-cdde055b7907

He uses Webpack to achieve the same as the above but I'm keen to keep the existing tool chain hence finding Gulp alternatives.

@nul800sebastiaan

This comment has been minimized.

Copy link
Member

commented Mar 4, 2019

Forgive me Pete, for I have sinned (I didn't read and try to understand this yet).

Can this be done in small bits, start with one of them, send a PR, we can look at it and encourage you, other people can take the model and help out, etc?

My AngularJS knowledge is tiny, so tell me if I'm saying something weird! :-)

@PeteDuncanson

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2019

Hehe, don't worry, I am taking it steady. This one "should" be a small step, its an additional Gulp command and should give some instant quick wins for all the files with minimum issues and then should "just work" for all.

Then I'll move on to another small step in another issue, I've got a few lined up but wanted to earn some rep with AngularJs in the core first and prove my worth...its been a while ;)

@nul800sebastiaan

This comment has been minimized.

Copy link
Member

commented Mar 4, 2019

Sounds lovely! 👍

@PeteDuncanson

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2019

Had a play with this one today. Some very promising results!

I've installed npmjs.com/package/gulp-angular-embed-templates and wired it into the processJS gulp task. That has embedded all the templates into the umbraco.directive.js bundle as part of the build. This means we don't need to pull down every template separately. As a bonus it minifies the templates too so even less bytes.

Download stats from Chrome as is with the current setup with debug=true as a benchmark:

Stats as is with separate templates

Stats with the templates embedded via this mod, again with debug=true:

Stats with templates embedded

0.5 seconds of the download time for the cost of an additional 100kb it total download. However that increase means every template for the back office is downloaded which should greatly speed up other interaction around the back off. Trade off worth doing I think.

These are the same stats but with debug=false for existing setup:

image

And Embedded templates:

image

Again we've save that same 0.5 second and reduced the number of requests by 19 while also downloading all the templates that the back office needs (or at least the ones it has declared in its directives) in one hit. That should speed up render times for all interactions from then on. Winning!

Had a click around and there are no issues I can see, everything just seems to "work".

PeteDuncanson added a commit to PeteDuncanson/Umbraco-CMS that referenced this issue Mar 5, 2019

Added gulp-angular-embed-template
Fixed umbraco#4845 by embedded the angularjs templates for directives in the directives, cuts down on network traffic and increases speed to render

@PeteDuncanson PeteDuncanson changed the title Inline AngularJS templates into directives and controllers V8: Inline AngularJS templates into directives and controllers Mar 22, 2019

PeteDuncanson added a commit to PeteDuncanson/Umbraco-CMS that referenced this issue Apr 25, 2019

@nielslyngsoe

This comment has been minimized.

Copy link
Contributor

commented Apr 26, 2019

Looks good.

Did you see any noticeable difference in compile times?

@nul800sebastiaan

This comment has been minimized.

Copy link
Member

commented Apr 26, 2019

Compile times are the same, so seems fine.

nul800sebastiaan added a commit that referenced this issue Apr 26, 2019

Added gulp-angular-embed-template (#4871)
Fixed #4845 by embedded the angularjs templates for directives in the directives, cuts down on network traffic and increases speed to render
@nul800sebastiaan

This comment has been minimized.

Copy link
Member

commented Jul 3, 2019

I've marked this as breaking since it will catch some people out who were using html interceptors before, as Nathan so well explained in his blog post: https://nathanw.com.au/post/replacing-interceptors-with-directive-injection/

@dawoe

This comment has been minimized.

Copy link
Member

commented Jul 26, 2019

Just stumbled on to this one. I see the html files for the directives are still shipped with umbraco. Now they are inline they can be removed from the install package

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.