MachUpskillingFY17 / JabbR-Core

Modern edition of JabbR chat client using .NET Core
MIT License
7 stars 8 forks source link

Refactor all javascript for local/cdn support and organization #44

Open adamtuliper opened 8 years ago

adamtuliper commented 8 years ago

asp.net core has functionality via tag helpers to go local or cdn based on environment dev/production/etc. Validate html for this. #43 did this in index.html, this will be an audit for all other html files - gathering this will be on our home and/or layout, but let's see..

Index.cshtml

<environment names="Development">
    <script src="~/lib/jquery/jquery-2.2.4.min.js" async></script>
</environment>
<environment names="Staging,Production">
    <script src="https://code.jquery.com/jquery-2.2.4.min.js"  integrity="sha256-BbhdlvQf/xTY9gja0Dq3HiwQF8LaCRTXxZKRutelT44=" crossorigin="anonymous"
            asp-fallback-src="~/lib/jquery/jquery-2.2.4.min.js"
            asp-fallback-test="window.jQuery" async>
    </script>
</environment>

Investigate everything in the scripts folder. These should all be organized into wwwroot/lib based on name except anything that is our own custom js, that can go in a new wwwroot/js

image

For the lib items, lets try to bundle them into a single file in /js ex wwwroot/js/vendor.min.js and all of our own code in wwwroot/js/site.min.js

I realize we first did CDN for jQuery, that was more of an example (unless David wants a change to use CDNs for everything - but lets organize and bundle for now). Maybe we want to bundle on build/project change? as of now it only does it on post-publish if I recall. Investigate that as well.

ashanhol commented 8 years ago

@adamtuliper is it a good idea to merge the adina/bundling-minification branch into the Adamt/devprodlinks branch and work from there?

Edit: Branches were merged in #91, #62, and #92. Will now branch from dev

ashanhol commented 8 years ago

After some extensive testing, I've found there are three notable issues. See current commit (442e130). 1. We want to organize scripts into cleaner folders under lib, however there are scripts that are not needed either in the project or in certain views. Therefore, globbing won't work until we clean up unneeded scripts or re-organize folders. 2. Bundling certain files are causing errors of various sorts. -We cannot bundle signalR. The project will break. Just explicitly declare it in index.cshtml. -The files jquery.pulse.js, jquery.KeyTips.js, jQuery.tmpl.min.js specifically, would consistently cause this error if bundled: Uncaught TypeError: (intermediate value)(intermediate value)(...) is not a function If you exclude them from the bundling and include them in index.cshtml there is no error. Possible issue with bundling extension. -Bundling jquery.cookie.js would cause the issue above in addition to: Uncaught TypeError: $.cookie is not a function. 3. I could not go further testing what specifically fails if I bundle our custom scripts. Even if I don't use bundled versions of all the scripts, I get an error in Chat.ui.js saying it "Cannot read property 'sort' of undefined", which is an anonymous function declared in the script itself. If i try to bundle all the scripts anyways it'll tell me "chat" is undefined.

adamtuliper commented 8 years ago
  1. Its prob best to be explicit into what we want to bundle anyways at this point, then we make sure we don't miss anything. Globs can be nice but then.. well you see what happened :)
  2. Why on SignalR? Any file should be able to be bundled IMHO. I wonder if the whole order issue is biting us here. Lets see a branch (push changes) with a simple repro (the first thing that causes it to break) and we can break it apart.
ashanhol commented 7 years ago

Update: minify-ing an already minified file was causing errors, the sort issue is being addressed in #94/#60, and signalr still has issues with being bundled.

ashanhol commented 7 years ago

Update to update: only chat.emoji.js and loader.js of the custom scripts can be bundled without either throwing errors or not making the page load.

adamtuliper commented 7 years ago

Need to test locally to find specific failing case before looping in Mads.