Skip to content
This repository has been archived by the owner on Mar 26, 2018. It is now read-only.

cdnify not doing anything. #955

Open
apps4u opened this issue Dec 18, 2014 · 26 comments · May be fixed by #1242
Open

cdnify not doing anything. #955

apps4u opened this issue Dec 18, 2014 · 26 comments · May be fixed by #1242

Comments

@apps4u
Copy link

apps4u commented Dec 18, 2014

Hi I'm new to angular and grunt . So please forgive me if Ive done something wrong . Ive created a angular app using the generator and Ive got the default grunt file only added two task one for ng-templates and one for less files. Now when I build with grunt I get a message saying the all angular files have been Cdnify but the vendor.js contains the angular.js files and weather I turn off cdnify or leave it on I get a vendor.js file of 877KB. So my understanding was the cdnify wold replace the bower local copy of angular.js files with a link to the google CDN but its tells me the all angular file have changed from bower to http google cdn but then with I look at the vendor.js file there is not link to the cdn versions of the files it concated and minify the local files can some one let me know If Im doing something wrong or that this feature is not working.
Thanks. :) :)

@apps4u
Copy link
Author

apps4u commented Dec 18, 2014

Ive tried to debug it it looks like cdnify added the link to google CDN but then when the scripts are sent to 'concat', and 'uglifyjs' it look like they get downloaded from google CDN and then concated into the vendor.js file if that the case I can not see why it would need the CDN version it can just use the local version ... Please forgive me if this is how its meant to work Im new to grunt and angular and I understood the CDN would mean that the scripts are loaded from CDN at run time.

@eddiemonge
Copy link
Member

honestly, i dont really know what cdnify is doing and it should probably be removed at some point.

@apps4u
Copy link
Author

apps4u commented Dec 19, 2014

ok I've found out it is changing the link to google CDN but when the concat and minify run they download the files from the CDN instead of leaving the CDN links in place as I edited my local bower version of jQuery and when running without Cdnify I got the edited version and when running with Cdnify I got the online version in my vendor.js file.

@alexdmejias
Copy link

Is there an update to this issue? I having the same result that @apps4u is having

@hanfeisun
Copy link

I agree, I believe cdnify did nothing but wasting time in this angular-generator, unless you move angular out of (though it will be too clumsy)

@eddiemonge
Copy link
Member

loading angular from the cdn would be nice but not always an option. not sure what to do here. might be a flag: load Angular from Google's CDN? but that might be a bit advanced.

@alexdmejias
Copy link

Is there an instance where Angular is currently loaded from a CDN? would it be possible to added the the config or do it on the dist target?

@eddiemonge
Copy link
Member

currently the generator doesnt load from the cdn. it should be a choice if the user wants to do it. might be better to remove the cdnify plugin and have a link on how to add it in along with support for loading angular from the cdn just to simplify things here.

@jmlivingston
Copy link

I accidentally found a workaround that seems to work. Once you've generated your app, open up app/index.html and ensure that the build scripts block (see below) is indented to the same as the vendor scripts above it, the references generated in dist/index.html is correct. Now try building again and you should see it referenced properly. I guess the build process is picky with tabs. Still not clear to me how to fix this in the generator.

<!-- build:js({.tmp,app}) scripts/scripts.js -->
<script src="scripts/app.js"></script>
<script src="scripts/controllers/main.js"></script>
<script src="scripts/controllers/about.js"></script>
<!-- endbuild -->

Note: I was getting an error for ng-annotate, so not sure if that's related either, but if this doesn't work try running the following at your generated app root directory: npm install grunt-ng-annotate --save-dev.

@pensierinmusica
Copy link
Contributor

My understanding is that:

  • "Dev" mode should have front-end modules installed locally through Bower, and automatically injected in index.html, therefore website reload is faster and is possible to work offline.
  • "Production" mode should instead replace in index.html the Bower installed modules with their corresponding most popular CDN references (whenever available).

Does this sound like a desirable scenario, or a possible feature we can all agree upon?

p.s. I can confirm that as of today (v 0.11.1) "grunt-google-cdn" (aka "cdnify") does absolutely nothing in the build process of "generator-angular". In fact, Bower installed modules get minified, concatenated, and injected in index.html as /scripts/vendor.js.

@stevemao
Copy link
Contributor

👍 for an option

@lianyi
Copy link

lianyi commented Mar 27, 2015

+1 to make it optional. It doesn't do anything ("version": "0.10.0") except adding extra time to cdnify files.

@pensierinmusica
Copy link
Contributor

Before creating this option, is there actually a way to make "cdnify" do what it is supposed to do?

@stevemao
Copy link
Contributor

stevemao commented Apr 1, 2015

Just remove wiredep, concat and uglify tasks (any tasks that modifies your bower deps)

@pensierinmusica
Copy link
Contributor

Isn't there a way of fixing it, changing the order of operations instead of removing some of them? Thx!

@ciccia
Copy link

ciccia commented Apr 14, 2015

+1 make it optional

@wthomsen
Copy link

My solution for this issue:

  1. Move cdnify up your build task list, just below wiredep
  2. Make cdnify run on app/index.html:
cdnify: {
      dist: {
        html: ['<%= yeoman.app %>/*.html']
      }
    },
  1. Use usemin's blockReplacements option to manually configure the string that replaces the usemin block in index.html:
// Performs rewrites based on filerev and the useminPrepare configuration
    usemin: {
      html: ['<%%= yeoman.dist %>/{,*/}*.html'],
      css: ['<%%= yeoman.dist %>/styles/{,*/}*.css'],
      options: {
        assetsDirs: [
          '<%%= yeoman.dist %>',
          '<%%= yeoman.dist %>/images',
          '<%%= yeoman.dist %>/styles'
        ],
        blockReplacements: {
          js: function (block) {
            var i, src, scripts = '';
            for (i = 0; i < block.src.length; i++) {
              src = block.src[i];
              // Append scripts that aren't loaded locally (i.e., replaced by cdnify)
              if (src.substring(0, 2) === '//') {
                scripts += '<script src="' + src + '"></script>';
              }
            }
            // Append the block's destination file
            scripts += '<script src="' + block.dest + '"></script>';
            return scripts;
          }
        }
      }
    },
  1. Re-run wiredep at the end of the build task list (to "undo" step 2)

Some gotchas:

  • You'll have to add a blockReplacements option for css if you have stylesheets being cdnify'd.
  • You have to prioritize the cdnify task so that vendor.js doesn't double-include your cdnify'd assets
  • Running cdnify on app/index.html and re-running wiredep is sloppy (ideally you run cdnify on a copy of index.html in .tmp, which requires additional reconfiguration of other build tasks), but as long as nothing outside of your usemin blocks is affected by cdnify, it's not an issue.

@pensierinmusica
Copy link
Contributor

Could we fix it in the original generator, so end users don't have to worry about complex custom configurations?

@stevemao
Copy link
Contributor

@pensierinmusica I'm sure if you submit a PR they will review it.

@eddiemonge
Copy link
Member

of course we will. I'd rather remove cdnify but fixing it is just as good, if not better

@brunomperes
Copy link

+1 Thanks @wthomsen for the workaround

I can submit a PR if the solution seems to be good enough

@wthomsen
Copy link

@brunomperes I'd advocate for a pull request if it:

  1. Included the matching code for CSS
  2. Acted on a copy of index.html (saved to .tmp) rather than on app/index.html (and re-running wiredep to undo the changes). With the current task flow, that ends up being more than just adding a copy task.

@brunomperes
Copy link

Yes, indeed.
I was taking a look at the code, I don't think I'll be able to catch up with it in practical time to submit a PR :/

@itsSaad
Copy link

itsSaad commented Nov 2, 2015

Still no fix for this?

@raphinesse raphinesse linked a pull request Dec 30, 2015 that will close this issue
@JoernBerkefeld
Copy link

+1 for making cdnify optional in the generation process
+2 for simply removing it.
+3 for doing anything at all about it

JoernBerkefeld added a commit to BitTubes/dashboard that referenced this issue Apr 8, 2016
@dubcdr
Copy link

dubcdr commented May 10, 2016

Is anything going on with this?
I see:

Running "cdnify:dist" (cdnify) task
Going through dist/404.html, dist/index.html to update script refs
✔ bower_components/angular/angular.js changed to //ajax.googleapis.com/ajax/libs/angularjs/1.5.0-beta.1/angular.min.js 
...

But it only has the scripts and vendor in the dist folder.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.