Why does bower task use path.join?

[anselmbradford]

AFAIK the path module is for joining two or more URL paths together, as shown here, however, in the lines here join is being used on only one path. Is this a mistake?

[anselmbradford]

How does this look instead for the layout option:

          layout: function(type, component) {
            var src = config.loc.src + '/vendor/';
            var dist = config.loc.dist + '/static/';
            if (type === 'img' || type === 'fonts') {
              return path.relative( src , dist + type );
            } else {
              return component;
            }
          }

[Scotchester]

You're right, there doesn't seem to be a need to join single items, but your proposed fix seems to be doing more stuff than just fixing that issue.

What's the idea behind creating that relative path?

[himedlooff]

join also normalizes the path.

[anselmbradford]

So the existing layout function is this:

          layout: function(type, component) {
            if (type === 'img') {
              return path.join('static/img');
            } else if (type === 'fonts') {
              return path.join('static/fonts');
            } else {
              return path.join(component);
            }
          }

The returned value from layout is a relative path from the target directory set in https://github.com/cfpb/generator-cf/blob/master/app/templates/bowerrc#L2. In the existing code, that means fonts and images end up in /dist/static/vendor/static/fonts and /dist/static/vendor/static/img. If I'm interpreting this correctly the idea is to actually move these to /dist/fonts and /dist/img, since the output CSS looks like:

@font-face {
  font-family: 'CFPB Minicons';
  src: url('../fonts/cf-icons.eot');
…

Even if these paths were correct a hardcoded value will only work while the bower install location and distribution location variable values (see here) remain unchanged, however if someone changes these variables without updating the task it will silently break.

Below is an updated script that creates a relative path that takes into account the loc.dist and bower install location values so that if those are updated the task will still work. I've also included a location variable for the vendor directory and an output log for moving of the static assets. Happy to open a PR to continue the discussion there?

/**
     * Set directory location variables.
     */
    loc: {
      src:  './src',
      dist: './dist',
      lib:  grunt.file.readJSON('./.bowerrc').directory,
    },

    /**
     * Bower: https://github.com/yatskevich/grunt-bower-task
     *
     * Migrate static assets to distribution directory.
     */
    bower: {
      process: {
        options: {
          targetDir: '<%= loc.lib %>',
          install: false,
          verbose: false,
          cleanTargetDir: false,
          layout: function(type, component, source) {
            var lib = config.loc.lib;
            var dist = config.loc.dist + '/static/';
            var assetPath = component;
            if (type === 'img' || type === 'fonts') {
              assetPath = path.relative( lib , dist + type );
              grunt.log.writeln('Moving ./' + source + ' to ' + dist + type);
            }
            return assetPath;
          }
        }
      }
    },

The output looks like the following:

(Note: this is from cfgov-refresh, which is using different directory location values at the moment—see https://github.com/cfpb/generator-cf/issues/73)

Running "bower:process" (bower) task
Moving ./src/static/vendor/cf-icons/src/fonts/cf-icons.eot to ./static/fonts
Moving ./src/static/vendor/cf-icons/src/fonts/cf-icons.svg to ./static/fonts
Moving ./src/static/vendor/cf-icons/src/fonts/cf-icons.ttf to ./static/fonts
Moving ./src/static/vendor/cf-icons/src/fonts/cf-icons.woff to ./static/fonts
Moving ./src/static/vendor/chosen/chosen-sprite.png to ./static/img
Moving ./src/static/vendor/chosen/chosen-sprite@2x.png to ./static/img
>> Copied packages to /Users/bradforda/Projects/cfgov-refresh/src/static/vendor

Done, without errors.

[himedlooff]

@anselmbradford I'm surprised that it's moving things to /dist/static/vendor/static/font, that was not the original behavior, although I have not kept up with the changes here. I like your modifications.

[contolini]

AFAIK these concerns have been addressed.