Modernizr/customizr

classPrefix is not working

Tokimon opened this issue ยท 14 comments

I have tried to create a build with the setting classPrefix: 'has-' from gulp, but the setting doesn't work, and as the gulp-modernizr is really just passing in the config to customizr, I write the issue here:

  1. The crawler doesn't find any properties has-xxx, so correct tests are not initialized.
  2. The prefix is never passed on to the Modernizr script.

I did some digging and it seems that the legacy property cssprefix is still used in the code:

  // customizr/src/crawler.js line 57
  // (method findStringMatches)

  prefix = settings.cssprefix || '';

Hence I need to pass in cssprefix: 'has-' instead of classPrefix: 'has-' before it works, but that contradicts the fact that the property name has been changed to classPrefix (see #8).

If I change the line indicated to prefix = settings.classPrefix || '' the crawler finds the correct properties with the has- prefix.

But unfortunately the prefix is still not passed on to the final Modernizr script and right now I have no precise idea why.

Below is the Gulp setting I have used:

  return gulp.src([path.resolve('.tmp/index.css'), path.resolve('.tmp/index.js')])
    .pipe(modernizr({
      "classPrefix": "has-",
      "options": [ "html5printshiv", "html5shiv", "setClasses" ],
    }))
    .pipe(uglify())
    .pipe(gulp.dest(config.destination));

As a side note; I think it is a bit strange that setClasses is not added to the default list of options, as personally I have never used Modernizr without wanting to use the classes as well.

Related issues
Modernizr/grunt-modernizr#30
#8

+1

Any news?

+1...

It seems this functionality is already merged into develop, not yet in master though :(

in customizr/src/builder.js on line 47 replace the following:

var modernizrOptions = {
    "feature-detects": tests,
    "options": options,
    "minify": minify,
    "dest": settings.dest
};

with:

var modernizrOptions = {
    "feature-detects": tests,
    "options": options,
    "minify": minify,
    "dest": settings.dest,
    "classPrefix": settings.classPrefix
};

now you can use it in your gulpfile like so:

.pipe(modernizr('modernizr-custom.js', {
    options : ['setClasses'],
    classPrefix: 'supports-'
})) 

Perfect. I will try it out ๐Ÿ‘

+1

thanks @dasblitz, you'll also need to update crawler.js at line 57 if you want to pick up the prefixed properties on crawl. Better yet, apply each of the changes that this commit makes (props to @michaelisjones in the above referenced issue)

+1! Apparently, the fix was already merged in the develop branch pull request #16 . Please also apply to master branch, so dependent packages/modules can also benefit this feature.

toh82 commented

Any update on this, seems it's still not included in the version with comes with npm install, could we make that happen, soon?

lvl99 commented

For the love of Jebus, someone, anybody, please merge this into master...

rejas commented

finally managed to get the fix merged and released in v1 @Tokimon can you check and close this issue?

Apparently I don't get my messages from github, so I only just saw this.
I no longer have the build set up with this, but I will reconstruct it and verify once I have some time.
But great job!