Refactor all pending activities to new pattern

IagoSRL commented 7 years ago

Current focus

Refactor all activities from 'legacy' pattern to new "folder-based, on-demand bundle, simplified, with disposable lifecycle". More details at this comment and issue #457, this blocks issue #788:

"folder-based": a folder per activity is created under /app/source/js/activities/ containing the JavaScript activity class (index.js) that autoregister itself in the app activities index, and contains all specific resources (template.html, style.styl). See examples already in the folder.
"on-demand bundle": all this activities are compiled with it's own content (plus external modules used only by the activity) into an individual js bundle, that the App knows how to load using the Shell loader when an activity URL is reached by the user.
"simplified": using components for content implementation, the activity simplifies just including general layout and navigation elements, processing URL parameters (routing), no needing a separated ViewModel class but using the own activity instance as the Knockout view model for the html.
"with disposable lifecycle": instances of the activity JavaScript class, DOM (from template.html) and styles are created each time user enters the activity and disposed (removed) when is abandoned. As opposite of legacy lifecycle whose DOM/html is pre-included on page load and once the class is instantiated is reused, requiring manual 'reset' logic each time is accessed.

Original comment and focus

Title: Investigate to separate public activities into it's own bundle

Initially commented at #203, we need to reduce the load time of the public site by separating the public activities html, js, css into it's own bundle.

The rest of them (basically, all user dashboard) will be kept into the current bundle.

The approach to investigate here is to follow what was done with landing pages; additional complexity here is that we need routing because we have several activities, and redirect the browser when the activity/url is not preloaded in the bundle.

If gets too complex/slow with current tooling, we can investigate the use of Webpack. As @situatedbit commented, we need too to investigate the implications of update browserify #360.

Related is #421: is not technically blocking us from creating a bundle, but it helps a lot with the main purpose of this issue (reducing load time).

situatedbit commented 7 years ago

How much time do you want to investigate this item?

IagoSRL commented 7 years ago

@situatedbit as advanced in the meeting, I was reviewing the option to 'require text' files from javascript, mainly for html files, to make dependency management for html files easier, mostly because of the /source/html/templates files. It affects this issue, too the Josh PR #463 to remove the manually inlined html.

At Webpack this seems a usual feature (something like require('text!path/to/an.html'), at Browserify there are some plugins to enable similar things:

brfs Allows to use Nodejs fs.readFileSync('') calls that are replaced when bundling with Browserify by inlining the text as javascript strings. A bit verbose.
textrequireify: Enables requireText('module/filename') syntax.
Stringify: Enables require('path/to/an.html'). Is based on extension matching.

Something of this lead me to this article 'Browserify vs Webpack'. Author is against overloading require or Nodejs incompatible code. Is from 2014 though.

Some thoughts?

IagoSRL commented 7 years ago

Forgot to add that one problem I see is we lost some processing on the html files (some custom task and the minifying).

situatedbit commented 7 years ago

I investigated the links you sent a little.

Webpack appears to be heavily supported and has a lot of momentum within the community. I suspect that in the long term it would be better for the project to be on webpack. However, I am unable to determine if the upfront cost of transitioning to Webpack away from Browserify is cost-effective right now.

I think we could load templates with require using stringify. It also supports html pre-processing, such as minify. I'm imagining we could

Load activity templates with stringify/require. It would be <script>...</script>
When the activity is initialized, then use jQuery to add that string to the DOM as a knockout template.
Then use knockout's named templates to bind the template from 2 to the main activity container.

We'd still have to remove any nested template definitions that we're using right now.

How would you like to move forward with this? I think using something like stringify might allow us to split code more quickly right now, but in the long term we should probably use webpack.

IagoSRL commented 7 years ago

I think similar about Webpack but choose a quick solution with Browserify now. Stringify looks the more complete, even I would prefer to being able to add our own preprocessing but seems it uses the same html-minifier module that we are using now already.

Another note: additional to remove templates from inside other html, any Bliss code ('static processing', the @render('../parts/something.html') snippets) need to be removed; may not be a problem though, that feature is expected to be used mostly at entry points pages (app.js.html, web.js.html) but we need to review and set a clear rule.

Let me test it later (cannot now) and answer back.

IagoSRL commented 7 years ago

@situatedbit I merged in branch changes with stringify installed, and in use at AppModel.paymentPlans.js

IagoSRL commented 7 years ago

Note that I moved setup of htmlmin to a file shared by the htmlmin and browserify tasks, so the stringify transform uses the same settings (an important one, remove html comments while keeping the Knockout special syntax).

IagoSRL commented 7 years ago

A detail, maybe minor, I noticed, is that by including html in js, every occurrence of double quotation marks (as in attributes) is escaped (" -> \"), so takes two characters rather than one, increasing a bit the file size versus the size of using html files.

Commenting about your process to move activities html into js, I'm thinking if rather than steps 2 and 3, the template can be placed with jQuery directly into the main activity container when needed.

IagoSRL commented 7 years ago

With #360 and #421 closed, would be possible to continue to work on this, but need to coordinate with changes proposed at #457. But there are other priorities and will move from In Progress.

joshdanielson commented 6 years ago

Adding #332 as a task for this issue:

[ ]
'
```
'
```

IagoSRL commented 6 years ago

Lot of progress on this issue: all base code to split every activity at it’s own bundle is in place (this changes a bit the scope of this issue, and goes from creating grouped activities bundles to individual, but all the details make it a better solution than initial approach). Details:

each activity bundle is just a JavaScript bundle, including html and css inside (originally expected one of each type)
every activity must register itself (similar to components) so is detected by the Shell after being loaded
the Shell has now a loader capable of get that bundles for any activity not yet loaded (once loaded get cached).
app.activities.js file will dissappear when this have completed.
only the ‘about’ activity is splitted right now, but is fully working (but please test and report anything!)
delay between user nagivating to an activity and that one loading by first time may need a loading indicator
some details of authoring activities have changed: organized by folders with owned ressources as style and template inside (same as with components); define template and style static getters in the class (same as with components)
all this changes are in master! Next is to adapt all the others activities; the system in place allows to have preloaded activities, I think just ‘home’ that is the front page, maybe learnMoreProfessionals too, but to change the list of included ones is easy (so we can fine tunning at any point).
further changes in the activity lifecycle (#457) are blocked by this, but some shared changes are in place
templates that exists under html/templates folder should be converted to kocomponents for better resuls in the objective of this issue (can be template-only components)

IagoSRL commented 6 years ago

Closed the previous issue branch is460-public-pages-bundle. New one should be created when continuing this, for anything else that is not done already as part of more specific feature or refactor issues.

IagoSRL commented 6 years ago

Since website/webapp loading performance is being really poor for a while, I choose to take some minutes to see if was possible to do a quick refactor of activities in order to get the benefit of get them split as new bundles with the new pattern.

Quick refactors

I started with 6 easy, dashboard index activities and didn't perform a depth refactor to don't loose time on less important code details, while have it working and isolated from the common bundle plus get it's resources out of loading from initial page load. I left the next comment on the top of the activities to know this situation and enable quick searching to finish the refactoring (usage of components, code docs and style, mostly): * TODO: Quick460 Must complete refactoring

Every of that worked fine and I merged already at master.

Heavy JPEGs

After that, I reviewed the performance loading of the site without cache, gets slightly better as expected, and looking the charts I realized that images was the most expensive resource being loading at every time and actually one activity loads most of them: the home activity. So the greater benefit comes from refactor just that (and learn-more-professionals that contains some JPEG images too). Note that there are some SVG images but are really small while the JPEGs are pretty big and a lot. In few minutes of quick refactor of this too activities, the page load performance cuts to 50% size and a bit more than the half of requests getting removed!

Much smaller "time to interactive" with lazy images

Of course, opening the site by going to the home page (that should be usual) it requires to load all it images still (if not cached), so keeps heavy but because this new start-up needs to initialize the home activity after load all main resources, it happens that automatically the images load lazily after the website going interactive so it's a much better experience even.

Of course I know we are going to replace the home page with the CMS but still is important to get that out for immediate performance benefit (this changes can be released today to live) and there is a chance that we keep this home activity with another name (like marketplace-search) when the new home/CMS is in place.

Current status

After this, the largest resource being loaded are the JavaScript bundles (2.4MB for common.js and 0.99MB for app.js), as I expected to be the case before and to reduce that heavily we need to complete this issue, and can be done with the quick refactor approach so takes shorter: basically, by doing the 1, 2 and 4 points in the bulled list at the top of this issue, being the number 3 more beneficial in terms of code complexity than size reduction while is more time consuming (getting it done right).

I think that task must be the main focus for this issue and keep that point 3 as another task to happen incrementally (the Quick460 comment will help identify activities that need that work).

IagoSRL commented 6 years ago

Forgot: page loading numbers

Before

Total requests: 34 Size: 8,849.65 KB

After

Total requests: 15 Size: 4,382.67 KB

IagoSRL commented 6 years ago

I merged and released latest changes to dev and live.

Page loading numbers at live

Production files, minified.
GA and other third party elements are enabled.
Desktop, high bandwidth connection (350MB).
Home loads lots of images
Dashboard triggers lot of data requests

loconomics / loconomics