jwhitley / requirejs-rails

RequireJS support for your Rails 3 or 4 application
MIT License
592 stars 202 forks source link

AssetSync uploading uncompiled module code #232

Closed benwalsh closed 9 years ago

benwalsh commented 9 years ago

Using flight with a collection of "per-page" files that were previously compiled via stand-alone RJS. I'm migrating to requirejs-rails. Each of the existing per-page files is now being wrapped in a template that pulls in jquery and a common package. (Some background in Issue #228)

Each per-page file then looks like this:

app/assets/javascripts/gr-page-home.js:

(function () {
  'use strict';
  require(['jquery', 'common', 'gr/pages/home'], function (_j, _c, initialize) {
    console.log('initializing page "home"!');
    initialize();
  });
}());

and requirejs.yml includes:

modules:
  - name: "gr-page-home"

Now when I deploy, the rake task runs and generates the "built" (and gzipped) versions of the per-page file I want:

-rw-rw-r-- 1 ubuntu eng 367972 Jun  2 23:32 20150602232425/public/assets/gr-page-home-9d310b9ded15807a502bebc1e6f9bf797539b72c5279cffdb604883db683137d.js
-rw-rw-r-- 1 ubuntu eng  97996 Jun  2 23:32 20150602232425/public/assets/gr-page-home-9d310b9ded15807a502bebc1e6f9bf797539b72c5279cffdb604883db683137d.js.gz

but the version that ends up on the CDN is the "source" file -- the 7 lines of original JS.

Is this an issue with how I'm configuring AssetSync, or how I'm configuring RequireJS-Rails, or both? (Or neither?)

I have been trying to hack the AssetSync rake task to change the order in which things happen or to move the compiled files around before continuing, but nothing has worked for me yet.

carsomyr commented 9 years ago

@benwalsh Thanks so much for looking into this! I read your description and am running through scenarios in my head.

carsomyr commented 9 years ago

@benwalsh Could you verify that these lines are being executed? This merges rjs_manifest.yml with the Rails manifest, generating a unified view. Could it be that AssetSync isn't picking up the unified manifest?

benwalsh commented 9 years ago

That's the magic I was trying to recreate on the AssetSync side -- blending the sprockets manifest and the RJS one.

 rails_manifest.assets.merge!(rjs_digests)
{
                                      "all.css" => "all-62670a16b69761e6620475a9dac39fbb5034a69a1790fb58460dd629b4d2146e.css",
                              "application.css" => "application-306432755622bcfd2b652a37a66cb1a299a53ec26a19fc577f9ea86cfe075a29.css",
                       "application_split2.css" => "application_split2-b5e83c89c3bd7c3deb8c770d2e5bfef6984541b055bece3cebe1f03c4cb7df0f.css",
                                   "buyers.css" => "buyers-1c99f367840bf4254efb39f90f48b709b7a9112b569eda0de1b8f109b08e72e6.css",
                            "buyers_split2.css" => "buyers_split2-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855.css",
                                    "email.css" => "email-08b6099b6211071d663a2fb580ab4327efc773a4502e0bf1088c7fcca998b16a.css",
                                    "other.css" => "other-aa74796f7cd7dde4b0fa0d62bfcf42035471b03f39a350c3afc5f582db77a150.css",
                                    "print.css" => "print-53fa4d465f85e5e755c509db042693357378a2bf7a52cf8608194c2f016a790c.css",
                             "read_content.css" => "read_content-8ba8d99bba133b4e60578eceee9ac9cce4b23b1fa8db2502efbeea41766b836c.css",
                   "lunr.js/example/require.js" => "lunr.js/example/require-992c62ca96a48688b34b135aeaa63f588500e09ddb55f12b1b83cd6d08f72853.js",
    "require-handlebars-plugin/demo/require.js" => "require-handlebars-plugin/demo/require-68bb33735efba086da0fdfacf86567748c580bb107ad77621103544285d7cab2.js",
                         "requirejs/require.js" => "requirejs/require-c9fd6919ecf6671a7f2d5a504ef01f6bf4a9db0fc8ea1a656a53382a397a2b0a.js",
             "requirejs-plugins/lib/require.js" => "requirejs-plugins/lib/require-b09dea18c776ebab53f2546a111a28a8144d7d002f4bace5d18f18c0b71af934.js",
                                   "require.js" => "require-c9fd6919ecf6671a7f2d5a504ef01f6bf4a9db0fc8ea1a656a53382a397a2b0a.js",
                              "gr-page-home.js" => "gr-page-home-9d310b9ded15807a502bebc1e6f9bf797539b72c5279cffdb604883db683137d.js"

But the file being syncd to S3 is still the "source" file located at app/assets/javascripts/gr-page-home.js and not the "compiled" one at public/assets/javascripts/gr-page-home-9d310b9ded15807a502bebc1e6f9bf797539b72c5279cffdb604883db683137d.js

Thanks for your help. This is tantalizingly close and is going to be so great when it gets there.

carsomyr commented 9 years ago

@benwalsh Ahhh, notice it's modifying the manifest attached to ActionView::Base, which ensures that the merged manifest works in views. The next step is to dig through AssetSync and figure out what manifest it's using.

benwalsh commented 9 years ago

It is possible that there's some user error at work here again. Because my source file had not changed over several iterations and configuration changes, the destination sha hadn't changed, the cache was not invalidated, so Cloudfront was still serving up the old version. I think it could be that simple ...

carsomyr commented 9 years ago

@benwalsh Any progress on this? AssetSync has been a thorn in my side for a while now, and its incompatible behavior has cropped up at least once before.

benwalsh commented 9 years ago

I had a caching issue that was the direct cause of this problem.

I have had to get a bit creative to skip the mini-manifest where the entire filemap is output to a script tag -- I only want one script file per page, so this is unnecessary, and we restrict inline script tags, so I've monkeypatched the requirejs_helper.rb file.

carsomyr commented 9 years ago

@benwalsh I may have identified the issue. Would you be so kind as to replace those two lines in question and just use assets_manifest directly? Also, verify that assets_manifest.assets is providing merged rjs_manifest.yml values.

carsomyr commented 9 years ago

@benwalsh Closing this issue, because I've file one upstream.