Skip to content

Commit

Permalink
Merge pull request #14736 from code-dot-org/lint-shared-js
Browse files Browse the repository at this point in the history
Start linting shared js, disallowing es6 syntax
  • Loading branch information
davidsbailey committed May 1, 2017
2 parents 476bc61 + 92bbd0f commit 7c0d72c
Show file tree
Hide file tree
Showing 11 changed files with 87 additions and 47 deletions.
11 changes: 0 additions & 11 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// or overridden for a particular subset of the project. See
// other .eslintrc.js files for those rules.
module.exports = {
"parser": "babel-eslint",
"plugins": [
"react",
"mocha",
Expand All @@ -16,16 +15,6 @@ module.exports = {
"browser": true,
"node": true,
"mocha": true,
"es6": true
},
"parserOptions": {
"sourceType": "module",
"ecmaFeatures": {
"jsx": true,
"modules": true,
"ecmaVersion": 6,
"experimentalObjectRestSpread": true
}
},
"rules": {
"array-bracket-spacing": ["error", "never"],
Expand Down
17 changes: 15 additions & 2 deletions apps/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,19 @@
// This config only defines globals available especially
// in apps. See the root .eslintrc.js for linting rules.
// This config only defines globals available especially in apps,
// and enables es6. See the root .eslintrc.js for linting rules.
module.exports = {
"parser": "babel-eslint",
"parserOptions": {
"sourceType": "module",
"ecmaFeatures": {
"jsx": true,
"modules": true,
"ecmaVersion": 6,
"experimentalObjectRestSpread": true
}
},
"env": {
"es6": true
},
"globals": {
"Blockly": true,
"Phaser": true,
Expand Down
2 changes: 1 addition & 1 deletion apps/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
"ejs-loader": "0.2.1",
"enzyme": "2.4.1",
"es6-promise": "3.0.2",
"eslint": "^3.7.1",
"eslint": "^3.19.0",
"eslint-plugin-mocha": "^4.9.0",
"eslint-plugin-react": "^5.2.2",
"exports-loader": "^0.6.3",
Expand Down
5 changes: 0 additions & 5 deletions apps/src/.eslintrc.js

This file was deleted.

28 changes: 21 additions & 7 deletions apps/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2146,7 +2146,7 @@ concat-stream@1.5.0:
readable-stream "~2.0.0"
typedarray "~0.0.5"

concat-stream@1.5.x, concat-stream@^1.4.1, concat-stream@^1.4.6, concat-stream@^1.4.7, concat-stream@^1.5.0:
concat-stream@1.5.x, concat-stream@^1.4.1, concat-stream@^1.4.7, concat-stream@^1.5.0, concat-stream@^1.5.2:
version "1.5.2"
resolved "https://registry.yarnpkg.com/concat-stream/-/concat-stream-1.5.2.tgz#708978624d856af41a5a741defdd261da752c266"
dependencies:
Expand Down Expand Up @@ -2747,6 +2747,13 @@ doctrine@^1.2.0, doctrine@^1.2.2:
esutils "^2.0.2"
isarray "^1.0.0"

doctrine@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/doctrine/-/doctrine-2.0.0.tgz#c73d8d2909d22291e1a007a395804da8b665fe63"
dependencies:
esutils "^2.0.2"
isarray "^1.0.0"

dom-helpers@^2.4.0:
version "2.4.0"
resolved "https://registry.yarnpkg.com/dom-helpers/-/dom-helpers-2.4.0.tgz#9bb4b245f637367b1fa670274272aa28fe06c367"
Expand Down Expand Up @@ -3162,17 +3169,18 @@ eslint-plugin-react@^5.2.2:
doctrine "^1.2.2"
jsx-ast-utils "^1.2.1"

eslint@^3.7.1:
version "3.16.1"
resolved "https://registry.yarnpkg.com/eslint/-/eslint-3.16.1.tgz#9bc31fc7341692cf772e80607508f67d711c5609"
eslint@^3.19.0:
version "3.19.0"
resolved "https://registry.yarnpkg.com/eslint/-/eslint-3.19.0.tgz#c8fc6201c7f40dd08941b87c085767386a679acc"
dependencies:
babel-code-frame "^6.16.0"
chalk "^1.1.3"
concat-stream "^1.4.6"
concat-stream "^1.5.2"
debug "^2.1.1"
doctrine "^1.2.2"
doctrine "^2.0.0"
escope "^3.6.0"
espree "^3.4.0"
esquery "^1.0.0"
estraverse "^4.2.0"
esutils "^2.0.2"
file-entry-cache "^2.0.0"
Expand Down Expand Up @@ -3232,6 +3240,12 @@ esprima@~3.1.0:
version "3.1.2"
resolved "https://registry.yarnpkg.com/esprima/-/esprima-3.1.2.tgz#954b5d19321ca436092fa90f06d6798531fe8184"

esquery@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/esquery/-/esquery-1.0.0.tgz#cfba8b57d7fba93f17298a8a006a04cda13d80fa"
dependencies:
estraverse "^4.0.0"

esrecurse@^4.1.0:
version "4.1.0"
resolved "https://registry.yarnpkg.com/esrecurse/-/esrecurse-4.1.0.tgz#4713b6536adf7f2ac4f327d559e7756bff648220"
Expand All @@ -3243,7 +3257,7 @@ estraverse@^1.9.1:
version "1.9.3"
resolved "https://registry.yarnpkg.com/estraverse/-/estraverse-1.9.3.tgz#af67f2dc922582415950926091a4005d29c9bb44"

estraverse@^4.1.1, estraverse@^4.2.0:
estraverse@^4.0.0, estraverse@^4.1.1, estraverse@^4.2.0:
version "4.2.0"
resolved "https://registry.yarnpkg.com/estraverse/-/estraverse-4.2.0.tgz#0dee3fed31fcd469618ce7342099fc1afa0bdb13"

Expand Down
4 changes: 4 additions & 0 deletions deployment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,10 @@ def shared_dir(*dirs)
deploy_dir('shared', *dirs)
end

def shared_js_dir(*dirs)
deploy_dir('shared/js', *dirs)
end

def lib_dir(*dirs)
deploy_dir('lib', *dirs)
end
8 changes: 5 additions & 3 deletions lib/rake/lint.rake
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ namespace :lint do
task :javascript do
Dir.chdir(apps_dir) do
ChatClient.log 'Linting <b>apps</b> JavaScript...'
# lint all js/jsx files in dashboard/app/assets/javascript
RakeUtils.system './node_modules/.bin/eslint -c .eslintrc.js ../dashboard/app/ --ext .js,.jsx'
# also do our standard apps lint
RakeUtils.system 'npm run lint'
end
Dir.chdir(shared_js_dir) do
ChatClient.log 'Linting <b>shared</b> JavaScript...'
# Use vanilla eslint parser, because babel-eslint always allows es6
RakeUtils.system '../../apps/node_modules/eslint/bin/eslint.js *.js'
end
end

task all: [:ruby, :haml, :scss, :javascript]
Expand Down
7 changes: 7 additions & 0 deletions shared/js/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module.exports = {
"parserOptions": {
"ecmaFeatures": {
"impliedStrict": true
}
}
}
30 changes: 15 additions & 15 deletions shared/js/angularProjects.js
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
/* global angular */
'use strict';
/* global $, angular */


// Declare app level module which depends on filters, and services
angular.module('projectsApp', [
'ngRoute',
'ngResource',
'projectsApp.controllers',
'projectsApp.services'
]).config(['$routeProvider', function($routeProvider) {
]).config(['$routeProvider', function ($routeProvider) {
$routeProvider.when('/',
{templateUrl: '/projects/angular', controller: 'ProjectsController'});
$routeProvider.otherwise({redirectTo: '/'});
}]);

// SERVICES
var services = angular.module('projectsApp.services', []).
value('version', '0.1');
var services = angular.module('projectsApp.services', [])
.value('version', '0.1');

// Section service. see sites.v3/code.org/routes/v2_section_routes.rb
services.factory('projectsService', ['$resource',
function($resource) {
function ($resource) {
var Project = $resource('/v3/channels/:id', {}, {
// default methods: see https://code.angularjs.org/1.2.21/docs/api/ngResource/service/$resource
// 'get': {method: 'GET'},
Expand All @@ -29,15 +29,15 @@ services.factory('projectsService', ['$resource',
// 'delete': {method: 'DELETE'} // don't use this because it doesn't work in IE9
});

Project.prototype.url = function() {
Project.prototype.url = function () {
if (this.level && this.id) {
return this.level.replace(/\/p\//, '/projects/') + '/' + this.id;
} else {
return null;
}
};

Project.prototype.editUrl = function() {
Project.prototype.editUrl = function () {
if (this.url()) {
return this.url() + "/edit";
} else {
Expand All @@ -50,11 +50,11 @@ services.factory('projectsService', ['$resource',

// CONTROLLERS

var controllers = angular.module('projectsApp.controllers', []).
value('version', '0.1');
var controllers = angular.module('projectsApp.controllers', [])
.value('version', '0.1');

controllers.controller('ProjectsController', ['$scope', '$route', '$routeParams', '$location', '$window', 'projectsService',
function($scope, $route, $routeParams, $location, $window, projectsService) {
function ($scope, $route, $routeParams, $location, $window, projectsService) {
$scope.projectsLoaded = false;

$scope.projects = projectsService.query();
Expand All @@ -63,20 +63,20 @@ controllers.controller('ProjectsController', ['$scope', '$route', '$routeParams'
$scope.order = 'updatedAt';
$scope.reverse = true;

$scope.projects.$promise.then(function(projects) {
$scope.projects.$promise.then(function (projects) {
$scope.projectsLoaded = true;
}).catch($scope.genericError);

$scope.projectVisible = function(project) {
$scope.projectVisible = function (project) {
return (!project.hidden);
};

$scope.genericError = function(result) {
$scope.genericError = function (result) {
$window.alert("An unexpected error occurred, please try again. If this keeps happening, try reloading the page.");
};

$scope.removeProject = function (project) {
project.$remove({id: project.id}, function() {
project.$remove({id: project.id}, function () {
$scope.projects.splice($.inArray(project, $scope.projects), 1);
});
};
Expand Down
3 changes: 3 additions & 0 deletions shared/js/helpers.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
/* global $ */
/* eslint-disable no-unused-vars */

function adjustScroll(destination) {
$('html, body').animate({
scrollTop: $("#" + destination).offset().top
Expand Down
19 changes: 16 additions & 3 deletions tools/hooks/lint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,22 @@

REPO_DIR = File.expand_path('../../../', __FILE__).freeze
APPS_DIR = "#{REPO_DIR}/apps".freeze
SHARED_JS_DIR = "#{REPO_DIR}/shared/js".freeze
SCSS_GLOB = "#{REPO_DIR}/#{YAML.load_file('.scss-lint.yml')['scss_files'] || '*'}".freeze

def filter_grunt_jshint(modified_files)
def filter_eslint_apps(modified_files)
modified_files.select do |f|
(f.end_with?(".js", ".jsx")) &&
!(f.end_with?('.min.js') || f.match(/public\/.+package\//) || f.match(/apps\/lib\//) || f.match(/shared\//))
end
end

def filter_eslint_shared(modified_files)
modified_files.select do |f|
f.match(%r{/shared/js/[^/]+.js})
end
end

RUBY_EXTENSIONS = ['.rake', '.rb', 'Rakefile'].freeze
def filter_rubocop(modified_files)
modified_rb_rake_files = modified_files.select do |f|
Expand Down Expand Up @@ -43,10 +50,15 @@ def run_rubocop(files)
run("bundle exec rubocop --force-exclusion #{files.join(' ')}", REPO_DIR)
end

def run_eslint(files)
def run_eslint_apps(files)
run("./node_modules/.bin/eslint -c .eslintrc.js #{files.join(' ')}", APPS_DIR)
end

def run_eslint_shared(files)
# Use vanilla eslint parser, because babel-eslint always allows es6
run("../../apps/node_modules/eslint/bin/eslint.js #{files.join(' ')}", SHARED_JS_DIR)
end

def run_haml(files)
run("bundle exec haml-lint #{files.join(' ')}", REPO_DIR)
end
Expand All @@ -66,7 +78,8 @@ def do_linting
todo = {
Object.method(:run_haml) => filter_haml(modified_files),
Object.method(:run_scss) => filter_scss(modified_files),
Object.method(:run_eslint) => filter_grunt_jshint(modified_files),
Object.method(:run_eslint_apps) => filter_eslint_apps(modified_files),
Object.method(:run_eslint_shared) => filter_eslint_shared(modified_files),
Object.method(:run_rubocop) => filter_rubocop(modified_files)
}

Expand Down

0 comments on commit 7c0d72c

Please sign in to comment.