Skip to content

Commit

Permalink
Add ES lint support to identify poorly written Promises
Browse files Browse the repository at this point in the history
  • Loading branch information
kushalpandya authored and Filipa Lacerda committed Apr 20, 2017
1 parent f96e1bf commit d586b6c
Show file tree
Hide file tree
Showing 21 changed files with 73 additions and 23 deletions.
6 changes: 4 additions & 2 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
"plugins": [
"filenames",
"import",
"html"
"html",
"promise"
],
"settings": {
"html/html-extensions": [".html", ".html.raw", ".vue"],
Expand All @@ -26,6 +27,7 @@
},
"rules": {
"filenames/match-regex": [2, "^[a-z0-9_]+$"],
"no-multiple-empty-lines": ["error", { "max": 1 }]
"no-multiple-empty-lines": ["error", { "max": 1 }],
"promise/catch-or-return": "error"
}
}
3 changes: 3 additions & 0 deletions app/assets/javascripts/awards_handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ AwardsHandler
if (menu) {
menu.dispatchEvent(new CustomEvent('build-emoji-menu-finish'));
}
}).catch((err) => {
emojiContentElement.insertAdjacentHTML('beforeend', '<p>We encountered an error while adding the remaining categories</p>');
throw new Error(`Error occurred in addRemainingEmojiMenuCategories: ${err.message}`);
});
};

Expand Down
3 changes: 2 additions & 1 deletion app/assets/javascripts/boards/boards_bundle.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable one-var, quote-props, comma-dangle, space-before-function-paren */
/* global BoardService */
/* global Flash */

import Vue from 'vue';
import VueResource from 'vue-resource';
Expand Down Expand Up @@ -93,7 +94,7 @@ $(() => {

Store.addBlankState();
this.loading = false;
});
}).catch(() => new Flash('An error occurred. Please try again.'));
},
methods: {
updateTokens() {
Expand Down
9 changes: 6 additions & 3 deletions app/assets/javascripts/boards/components/board_list.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,15 @@ export default {
},
loadNextPage() {
const getIssues = this.list.nextPage();
const loadingDone = () => {
this.list.loadingMore = false;
};

if (getIssues) {
this.list.loadingMore = true;
getIssues.then(() => {
this.list.loadingMore = false;
});
getIssues
.then(loadingDone)
.catch(loadingDone);
}
},
toggleForm() {
Expand Down
16 changes: 10 additions & 6 deletions app/assets/javascripts/boards/components/modal/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,13 @@ gl.issueBoards.IssuesModal = Vue.extend({
showAddIssuesModal() {
if (this.showAddIssuesModal && !this.issues.length) {
this.loading = true;
const loadingDone = () => {
this.loading = false;
};

this.loadIssues()
.then(() => {
this.loading = false;
});
.then(loadingDone)
.catch(loadingDone);
} else if (!this.showAddIssuesModal) {
this.issues = [];
this.selectedIssues = [];
Expand All @@ -67,11 +69,13 @@ gl.issueBoards.IssuesModal = Vue.extend({
if (this.$el.tagName) {
this.page = 1;
this.filterLoading = true;
const loadingDone = () => {
this.filterLoading = false;
};

this.loadIssues(true)
.then(() => {
this.filterLoading = false;
});
.then(loadingDone)
.catch(loadingDone);
}
},
deep: true,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable comma-dangle, func-names, no-new, space-before-function-paren, one-var */
/* eslint-disable comma-dangle, func-names, no-new, space-before-function-paren, one-var,
promise/catch-or-return */

window.gl = window.gl || {};
window.gl.issueBoards = window.gl.issueBoards || {};
Expand Down
3 changes: 3 additions & 0 deletions app/assets/javascripts/boards/stores/boards_store.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ gl.issueBoards.BoardsStore = {
.save()
.then(() => {
this.state.lists = _.sortBy(this.state.lists, 'position');
})
.catch(() => {
// https://gitlab.com/gitlab-org/gitlab-ce/issues/30821
});
this.removeBlankState();
},
Expand Down
6 changes: 5 additions & 1 deletion app/assets/javascripts/diff_notes/components/resolve_btn.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ const ResolveBtn = Vue.extend({
});
},
resolve: function () {
const errorFlashMsg = 'An error occurred when trying to resolve a comment. Please try again.';

if (!this.canResolve) return;

let promise;
Expand All @@ -87,10 +89,12 @@ const ResolveBtn = Vue.extend({
CommentsStore.update(this.discussionId, this.noteId, !this.isResolved, resolved_by);
this.discussion.updateHeadline(data);
} else {
new Flash('An error occurred when trying to resolve a comment. Please try again.', 'alert');
new Flash(errorFlashMsg);
}

this.updateTooltip();
}).catch(() => {
new Flash(errorFlashMsg);
});
}
},
Expand Down
4 changes: 3 additions & 1 deletion app/assets/javascripts/diff_notes/services/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,10 @@ class ResolveServiceClass {

discussion.updateHeadline(data);
} else {
new Flash('An error occurred when trying to resolve a discussion. Please try again.', 'alert');
throw new Error('An error occurred when trying to resolve discussion.');
}
}).catch(() => {
new Flash('An error occurred when trying to resolve a discussion. Please try again.');
});
}

Expand Down
8 changes: 5 additions & 3 deletions app/assets/javascripts/due_date_select.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,13 @@ class DueDateSelect {
this.$dropdown.trigger('loading.gl.dropdown');
this.$selectbox.hide();
this.$value.css('display', '');
const fadeOutLoader = () => {
this.$loading.fadeOut();
};

gl.issueBoards.BoardsStore.detail.issue.update(this.$dropdown.attr('data-issue-update'))
.then(() => {
this.$loading.fadeOut();
});
.then(fadeOutLoader)
.catch(fadeOutLoader);
}

submitSelectedDate(isDropdown) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,8 @@ class FilteredSearchManager {
const resultantSearches = this.recentSearchesStore.addRecentSearch(searchQuery);
this.recentSearchesService.save(resultantSearches);
}
}).catch(() => {
// https://gitlab.com/gitlab-org/gitlab-ce/issues/30821
});
}

Expand Down
6 changes: 5 additions & 1 deletion app/assets/javascripts/groups_select.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
/* eslint-disable func-names, space-before-function-paren, no-var, wrap-iife, one-var, camelcase, one-var-declaration-per-line, quotes, object-shorthand, prefer-arrow-callback, comma-dangle, consistent-return, yoda, prefer-rest-params, prefer-spread, no-unused-vars, prefer-template, max-len */
/* eslint-disable func-names, space-before-function-paren, no-var, wrap-iife, one-var,
camelcase, one-var-declaration-per-line, quotes, object-shorthand,
prefer-arrow-callback, comma-dangle, consistent-return, yoda,
prefer-rest-params, prefer-spread, no-unused-vars, prefer-template,
promise/catch-or-return */
/* global Api */

var slice = [].slice;
Expand Down
8 changes: 5 additions & 3 deletions app/assets/javascripts/labels_select.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,9 @@
vue: $dropdown.hasClass('js-issue-board-sidebar'),
clicked: function(label, $el, e, isMarking) {
var isIssueIndex, isMRIndex, page, boardsModel;
var fadeOutLoader = () => {
$loading.fadeOut();
};

page = $('body').data('page');
isIssueIndex = page === 'projects:issues:index';
Expand Down Expand Up @@ -396,9 +399,8 @@
$loading.fadeIn();

gl.issueBoards.BoardsStore.detail.issue.update($dropdown.attr('data-issue-update'))
.then(function () {
$loading.fadeOut();
});
.then(fadeOutLoader)
.catch(fadeOutLoader);
}
else {
if ($dropdown.hasClass('js-multiselect')) {
Expand Down
3 changes: 3 additions & 0 deletions app/assets/javascripts/milestone_select.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,9 @@
.then(function () {
$dropdown.trigger('loaded.gl.dropdown');
$loading.fadeOut();
})
.catch(() => {
$loading.fadeOut();
});
} else {
selected = $selectbox.find('input[type="hidden"]').val();
Expand Down
2 changes: 2 additions & 0 deletions app/assets/javascripts/monitoring/prometheus_graph.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ class PrometheusGraph {
this.transformData(metricsResponse);
this.createGraph();
}
}).catch(() => {
new Flash('An error occurred when trying to load metrics. Please try again.');
});
}

Expand Down
3 changes: 3 additions & 0 deletions app/assets/javascripts/users_select.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@
gl.issueBoards.BoardsStore.detail.issue.update($dropdown.attr('data-issue-update'))
.then(function () {
$loading.fadeOut();
})
.catch(function () {
$loading.fadeOut();
});
};

Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"eslint-plugin-filenames": "^1.1.0",
"eslint-plugin-import": "^2.2.0",
"eslint-plugin-jasmine": "^2.1.0",
"eslint-plugin-promise": "^3.5.0",
"istanbul": "^0.4.5",
"jasmine-core": "^2.5.2",
"jasmine-jquery": "^2.1.1",
Expand Down
2 changes: 1 addition & 1 deletion spec/javascripts/blob/sketch/index_spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* eslint-disable no-new */
/* eslint-disable no-new, promise/catch-or-return */
import JSZip from 'jszip';
import SketchLoader from '~/blob/sketch';

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* eslint-disable promise/catch-or-return */

import RecentSearchesService from '~/filtered_search/services/recent_searches_service';

describe('RecentSearchesService', () => {
Expand Down
2 changes: 2 additions & 0 deletions spec/javascripts/lib/utils/common_utils_spec.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* eslint-disable promise/catch-or-return */

require('~/lib/utils/common_utils');

(() => {
Expand Down
4 changes: 4 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1988,6 +1988,10 @@ eslint-plugin-jasmine@^2.1.0:
version "2.2.0"
resolved "https://registry.yarnpkg.com/eslint-plugin-jasmine/-/eslint-plugin-jasmine-2.2.0.tgz#7135879383c39a667c721d302b9f20f0389543de"

eslint-plugin-promise@^3.5.0:
version "3.5.0"
resolved "https://registry.yarnpkg.com/eslint-plugin-promise/-/eslint-plugin-promise-3.5.0.tgz#78fbb6ffe047201627569e85a6c5373af2a68fca"

eslint@^3.10.1:
version "3.15.0"
resolved "https://registry.yarnpkg.com/eslint/-/eslint-3.15.0.tgz#bdcc6a6c5ffe08160e7b93c066695362a91e30f2"
Expand Down

0 comments on commit d586b6c

Please sign in to comment.