skilld-labs / zen

Fork of Drupal Zen theme https://www.drupal.org/project/zen
1 stars 5 forks source link

Gulp/Sass: Improvements #4

Closed pabloguerino closed 7 years ago

pabloguerino commented 7 years ago

Proper import resolvers

We can update this and have only 1 function to handle the resolution of the imports: https://github.com/skilld-labs/zen/blob/8.x-7.x/STARTERKIT/gulpfile.js#L47

We have better examples already that we can apply here

Use production flag / detect environment

Apply correct manipulations depending on environment (source maps, minification, compression, etc)

Do not repeat tasks

https://github.com/skilld-labs/zen/blob/8.x-7.x/STARTERKIT/gulpfile.js#L143 https://github.com/skilld-labs/zen/blob/8.x-7.x/STARTERKIT/gulpfile.js#L156

https://github.com/skilld-labs/zen/blob/8.x-7.x/STARTERKIT/gulpfile.js#L208 https://github.com/skilld-labs/zen/blob/8.x-7.x/STARTERKIT/gulpfile.js#L215

https://github.com/skilld-labs/zen/blob/8.x-7.x/STARTERKIT/gulpfile.js#L193 https://github.com/skilld-labs/zen/blob/8.x-7.x/STARTERKIT/gulpfile.js#L200

we could just use gulp if instead, based on the environment https://github.com/robrich/gulp-if

pabloguerino commented 7 years ago

Proper import resolvers

Now imports are based on node package module name, so Instead of

// Define the node-sass configuration. The includePaths is critical!
options.sass = {
  importer: importOnce,
  includePaths: [
    options.theme.components,
    options.rootPath.project + 'node_modules/breakpoint-sass/stylesheets',
    options.rootPath.project + 'node_modules/chroma-sass/sass',
    options.rootPath.project + 'node_modules/support-for/sass',
    options.rootPath.project + 'node_modules/typey/stylesheets',
    options.rootPath.project + 'node_modules/zen-grids/sass'
  ],
  outputStyle: 'expanded'
};

We now have:

function sassModuleImporter(url, file, done) {
  try {
    let pathResolution = require.resolve(url);
    return done({ file: pathResolution });
  }
  catch (e) {
    return null;
  }
}

// Define the node-sass configuration. The includePaths is critical!
options.sass = {
  importer: [sassModuleImporter, importOnce],
  includePaths: options.theme.components,
  outputStyle: 'expanded'
};

Repeated tasks and environment management

styles:production has been removed and an environment variable has been added NODE_ENV which can have the value of production, development and testing for CI. No need for extra tasks for the linters anymore.

for the same reason, lint:js-with-fail and lint:sass-with-fail are no longer needed and will fail on errors when NODE_ENV=testing

andypost commented 7 years ago

Needs README updated from #5

pabloguerino commented 7 years ago

@andypost Will create a new pull request later to update readme