peerigon/modernizr-loader

After update to 1.0.0 detects are incorrect

Closed this issue ยท 12 comments

After the update from 0.5 some feature detects are incorrect, for example with a project in 0.5 the feature detect "eventlistener" returns true on the latest version of Chrome, with 1.0.0 it returns false.

Hey @amorino, thanks for reporting this bug. Could you please provide us with some more information about your webpack and modernizr configuration? This would be helpful, thanks!

@flootr I have anther issue using v1.0.0. I'm chaining yaml-loader and modernizr-loader to write the config file in yaml. Here's my .modernizerrc

minify: false

options:
  - setClasses
  # - testAllProps
  # - mq

feature-detects:
  # - svg
  # - video

After starting webpack-dev-server, I got this error:

ERROR in ./.modernizrrc
Module build failed: /Users/jude/sites/devcore2017/.modernizrrc:2
	"minify": false,
	        ^
SyntaxError: Unexpected token :
    at Object.exports.runInThisContext (vm.js:76:16)
    at Module._compile (module.js:542:28)
    at Object.exec (/Users/jude/sites/devcore2017/node_modules/webpack-core/lib/NormalModuleMixin.js:88:7)
    at Object.module.exports (/Users/jude/sites/devcore2017/node_modules/modernizr-loader/index.js:20:26)
 @ ./client/index.coffee 5:0-20
webpack: bundle is now VALID.
^C

I have tried the same setup with v0.0.5, everything worked great.

@judewang you could try to pass in the json-loader before passing stuff from the yaml-loader to the modernizr-loader. Like so: modernizr!json!yaml.

@flootr It worked! I haven't notice that new version needs json-loader, thanks.

Sorry to pile-on a somewhat unrelated issue, but it might not be a bad idea to make a note about upgrade path:

  1. Install json-loader when using JSON config.
  2. Add !json to loader chain in Webpack config.
  3. Properly import Modernizr where previously you'd been relying on a global.

I seem to be up and running again, thanks for your work!

@synapticism good point! I updated the CHANGELOG accordingly.

I believe this bug is caused by #23. In the built Modernizr file, window will be an empty object now, so tests such as "addEventListener" in window return false.

I took a closer look and it seems I'm also experiencing the same problem as the OP: multiple tests are failing where the global window object is referenced in the Modernizr test.

I created a PR #25 which should resolve the issue.

Can we merge that pull request? Works properly.

@clessg thanks for your PR ๐Ÿ‘

I somehow want to have this tested though. I'm going to set up a local test project to see how we can improve on this.

@flootr when can we merge this PR, getting the same issue as some modernizr tests are failing. Thanks