csantero / ember-sinon

Ember CLI addon adding support for sinon.js
MIT License
64 stars 13 forks source link

jshint config for sinon as a global #6

Closed michaelBenin closed 9 years ago

michaelBenin commented 9 years ago

I've updated my jshintrc and it still complained sinon was a global.

I don't know if this is an issue with this project or ember-cli.

Here's some background:

jshintrc:

{
  "predef": {
    "document": true,
    "window": true,
    "sinon": true
  },
  "browser": true,
  "boss": true,
  "curly": true,
  "debug": false,
  "devel": true,
  "eqeqeq": true,
  "evil": true,
  "forin": false,
  "immed": false,
  "laxbreak": false,
  "newcap": true,
  "noarg": true,
  "noempty": false,
  "nonew": false,
  "nomen": false,
  "onevar": false,
  "plusplus": false,
  "regexp": false,
  "undef": true,
  "sub": true,
  "strict": false,
  "white": false,
  "eqnull": true,
  "esnext": true,
  "unused": true
}

package.json:

{
  "name": "equipmentwatch-ember",
  "version": "0.0.1",
  "description": "Small description for equipmentwatch-ember goes here",
  "private": true,
  "directories": {
    "doc": "doc",
    "test": "tests"
  },
  "scripts": {
    "start": "ember server --proxy http://localhost:8001",
    "build": "ember build",
    "test": "ember test",
    "build-dev": "ember build",
    "build-qa": "ember build --environment production",
    "build-staging": "ember build --environment production",
    "build-prod": "ember build --environment production",
    "precommit": "npm test"
  },
  "repository": "",
  "engines": {
    "node": ">= 0.10.0"
  },
  "author": "",
  "license": "MIT",
  "devDependencies": {
    "broccoli-asset-rev": "^2.0.0",
    "ember-cli": "0.2.0-beta.1",
    "ember-cli-app-version": "^0.3.2",
    "ember-cli-babel": "^4.1.0",
    "ember-cli-content-security-policy": "0.3.0",
    "ember-cli-dependency-checker": "0.0.8",
    "ember-cli-htmlbars": "^0.7.4",
    "ember-cli-ic-ajax": "0.1.1",
    "ember-cli-inject-live-reload": "^1.3.0",
    "ember-cli-qunit": "^0.3.9",
    "ember-cli-sass": "3.1.0",
    "ember-cli-simple-auth": "0.7.3",
    "ember-cli-uglify": "1.0.1",
    "ember-cli-yuidoc": "0.6.0",
    "ember-data": "^1.0.0-beta.15",
    "ember-export-application-global": "^1.0.0",
    "ember-sinon": "0.0.3",
    "ember-validations": "2.0.0-alpha.2",
    "express": "^4.8.5",
    "glob": "^4.0.5",
    "husky": "^0.6.2",
    "rimraf": "^2.3.2"
  }
}

Line of code causing the issue:

  var callback = sinon.spy();

I don't want to do:

var sinon = window.sinon;
csantero commented 9 years ago

Not sure why the predefs aren't working. At the very least this should work if you put it at the top of any file that uses sinon:

/* global sinon */
michaelBenin commented 9 years ago

For now yes we can solve this with a comment but I'd prefer if the jshint config persisted these changes.

Do you think I should look into escalating this to ember-cli?

csantero commented 9 years ago

I just tried this out myself and I wasn't able to reproduce the problem. I created a new app, added "sinon" to the predef of /tests/.jshintrc, then used sinon in one of the tests, and jshint was happy. Are you sure you edited the right .jshintrc file?

michaelBenin commented 9 years ago

Ah ok, wasn't aware there was a /tests/.jshintrc. I was under the impression the only .jshintrc was in the root of the directory.

csantero commented 9 years ago

Yeah, I always felt that was confusing for that reason. IMO the root-level .jshintrc should either not exist or at least be a node-style one (for the Brocfile). And /tests and /app should have their own browser versions.

michaelBenin commented 9 years ago

Is it possible to update the tests/.jshintrc file on install?

csantero commented 9 years ago

Yeah it could be part of the ember-sinon blueprint. I don't have the bandwidth for this right now, but PRs are welcome!

Of course the ideal situation is for ember CLI to enable importing anonymous AMD modules as ES6 ones, so you can avoid globals altogether.

sukima commented 9 years ago

@michaelBenin: predef should be an array not an object AFAIK:

  "predef": [
    "document",
    "window",
    "sinon"
  ],

@csantero:

Of course the ideal situation is for ember CLI to enable importing anonymous AMD modules as ES6 ones, so you can avoid globals altogether.

If only, I have my doubts it will ever happen. ES6 Modules can not come sooner enough!

csantero commented 9 years ago

Closed via https://github.com/csantero/ember-sinon/commit/e42b6b82e2f36435468a71631e3c4f60a4f15937. Decided to use a shim module to make sinon available as an import, inspired by ember-cli-pretender.