From d586b6cced34d663f018203e99aecbb9c6ac3158 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Thu, 20 Apr 2017 11:11:51 +0000 Subject: [PATCH] Add ES lint support to identify poorly written Promises --- .eslintrc | 6 ++++-- app/assets/javascripts/awards_handler.js | 3 +++ app/assets/javascripts/boards/boards_bundle.js | 3 ++- .../javascripts/boards/components/board_list.js | 9 ++++++--- .../javascripts/boards/components/modal/index.js | 16 ++++++++++------ .../boards/components/new_list_dropdown.js | 3 ++- .../javascripts/boards/stores/boards_store.js | 3 +++ .../diff_notes/components/resolve_btn.js | 6 +++++- .../javascripts/diff_notes/services/resolve.js | 4 +++- app/assets/javascripts/due_date_select.js | 8 +++++--- .../filtered_search/filtered_search_manager.js | 2 ++ app/assets/javascripts/groups_select.js | 6 +++++- app/assets/javascripts/labels_select.js | 8 +++++--- app/assets/javascripts/milestone_select.js | 3 +++ .../javascripts/monitoring/prometheus_graph.js | 2 ++ app/assets/javascripts/users_select.js | 3 +++ package.json | 1 + spec/javascripts/blob/sketch/index_spec.js | 2 +- .../services/recent_searches_service_spec.js | 2 ++ spec/javascripts/lib/utils/common_utils_spec.js | 2 ++ yarn.lock | 4 ++++ 21 files changed, 73 insertions(+), 23 deletions(-) diff --git a/.eslintrc b/.eslintrc index 57a08a06527..aba8112c5a9 100644 --- a/.eslintrc +++ b/.eslintrc @@ -14,7 +14,8 @@ "plugins": [ "filenames", "import", - "html" + "html", + "promise" ], "settings": { "html/html-extensions": [".html", ".html.raw", ".vue"], @@ -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" } } diff --git a/app/assets/javascripts/awards_handler.js b/app/assets/javascripts/awards_handler.js index f93208944a1..adb45b0606d 100644 --- a/app/assets/javascripts/awards_handler.js +++ b/app/assets/javascripts/awards_handler.js @@ -239,6 +239,9 @@ AwardsHandler if (menu) { menu.dispatchEvent(new CustomEvent('build-emoji-menu-finish')); } + }).catch((err) => { + emojiContentElement.insertAdjacentHTML('beforeend', '

We encountered an error while adding the remaining categories

'); + throw new Error(`Error occurred in addRemainingEmojiMenuCategories: ${err.message}`); }); }; diff --git a/app/assets/javascripts/boards/boards_bundle.js b/app/assets/javascripts/boards/boards_bundle.js index b749ef43cd3..b6dee8177d2 100644 --- a/app/assets/javascripts/boards/boards_bundle.js +++ b/app/assets/javascripts/boards/boards_bundle.js @@ -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'; @@ -93,7 +94,7 @@ $(() => { Store.addBlankState(); this.loading = false; - }); + }).catch(() => new Flash('An error occurred. Please try again.')); }, methods: { updateTokens() { diff --git a/app/assets/javascripts/boards/components/board_list.js b/app/assets/javascripts/boards/components/board_list.js index adbd82cb687..b13386536bf 100644 --- a/app/assets/javascripts/boards/components/board_list.js +++ b/app/assets/javascripts/boards/components/board_list.js @@ -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() { diff --git a/app/assets/javascripts/boards/components/modal/index.js b/app/assets/javascripts/boards/components/modal/index.js index fb0aac3c0e4..fdab317dc23 100644 --- a/app/assets/javascripts/boards/components/modal/index.js +++ b/app/assets/javascripts/boards/components/modal/index.js @@ -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 = []; @@ -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, diff --git a/app/assets/javascripts/boards/components/new_list_dropdown.js b/app/assets/javascripts/boards/components/new_list_dropdown.js index 22f20305624..7e3bb79af1d 100644 --- a/app/assets/javascripts/boards/components/new_list_dropdown.js +++ b/app/assets/javascripts/boards/components/new_list_dropdown.js @@ -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 || {}; diff --git a/app/assets/javascripts/boards/stores/boards_store.js b/app/assets/javascripts/boards/stores/boards_store.js index 66384d9c038..ccb00099215 100644 --- a/app/assets/javascripts/boards/stores/boards_store.js +++ b/app/assets/javascripts/boards/stores/boards_store.js @@ -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(); }, diff --git a/app/assets/javascripts/diff_notes/components/resolve_btn.js b/app/assets/javascripts/diff_notes/components/resolve_btn.js index 8fafd13c6c2..92f6fd654b3 100644 --- a/app/assets/javascripts/diff_notes/components/resolve_btn.js +++ b/app/assets/javascripts/diff_notes/components/resolve_btn.js @@ -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; @@ -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); }); } }, diff --git a/app/assets/javascripts/diff_notes/services/resolve.js b/app/assets/javascripts/diff_notes/services/resolve.js index e1e2e3e93f9..4ea6ba8a73d 100644 --- a/app/assets/javascripts/diff_notes/services/resolve.js +++ b/app/assets/javascripts/diff_notes/services/resolve.js @@ -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.'); }); } diff --git a/app/assets/javascripts/due_date_select.js b/app/assets/javascripts/due_date_select.js index db10b383913..dd95b6530f6 100644 --- a/app/assets/javascripts/due_date_select.js +++ b/app/assets/javascripts/due_date_select.js @@ -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) { diff --git a/app/assets/javascripts/filtered_search/filtered_search_manager.js b/app/assets/javascripts/filtered_search/filtered_search_manager.js index a5eb33dd9de..68a832102a0 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_manager.js +++ b/app/assets/javascripts/filtered_search/filtered_search_manager.js @@ -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 }); } diff --git a/app/assets/javascripts/groups_select.js b/app/assets/javascripts/groups_select.js index 10363c16bae..acfa4bd4c6b 100644 --- a/app/assets/javascripts/groups_select.js +++ b/app/assets/javascripts/groups_select.js @@ -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; diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index 443fb3e0ca9..9a60f5464df 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -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'; @@ -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')) { diff --git a/app/assets/javascripts/milestone_select.js b/app/assets/javascripts/milestone_select.js index 773fe3233a7..bebd0aa357e 100644 --- a/app/assets/javascripts/milestone_select.js +++ b/app/assets/javascripts/milestone_select.js @@ -164,6 +164,9 @@ .then(function () { $dropdown.trigger('loaded.gl.dropdown'); $loading.fadeOut(); + }) + .catch(() => { + $loading.fadeOut(); }); } else { selected = $selectbox.find('input[type="hidden"]').val(); diff --git a/app/assets/javascripts/monitoring/prometheus_graph.js b/app/assets/javascripts/monitoring/prometheus_graph.js index d82a4eb9642..aff507abb91 100644 --- a/app/assets/javascripts/monitoring/prometheus_graph.js +++ b/app/assets/javascripts/monitoring/prometheus_graph.js @@ -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.'); }); } diff --git a/app/assets/javascripts/users_select.js b/app/assets/javascripts/users_select.js index 3325a7d429c..30902767705 100644 --- a/app/assets/javascripts/users_select.js +++ b/app/assets/javascripts/users_select.js @@ -56,6 +56,9 @@ gl.issueBoards.BoardsStore.detail.issue.update($dropdown.attr('data-issue-update')) .then(function () { $loading.fadeOut(); + }) + .catch(function () { + $loading.fadeOut(); }); }; diff --git a/package.json b/package.json index a17399ddb8f..e65f30eea77 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/spec/javascripts/blob/sketch/index_spec.js b/spec/javascripts/blob/sketch/index_spec.js index 0e4431548c4..79f40559817 100644 --- a/spec/javascripts/blob/sketch/index_spec.js +++ b/spec/javascripts/blob/sketch/index_spec.js @@ -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'; diff --git a/spec/javascripts/filtered_search/services/recent_searches_service_spec.js b/spec/javascripts/filtered_search/services/recent_searches_service_spec.js index 2a58fb3a7df..c255bf7c939 100644 --- a/spec/javascripts/filtered_search/services/recent_searches_service_spec.js +++ b/spec/javascripts/filtered_search/services/recent_searches_service_spec.js @@ -1,3 +1,5 @@ +/* eslint-disable promise/catch-or-return */ + import RecentSearchesService from '~/filtered_search/services/recent_searches_service'; describe('RecentSearchesService', () => { diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index 56aabc16382..a00efa10119 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -1,3 +1,5 @@ +/* eslint-disable promise/catch-or-return */ + require('~/lib/utils/common_utils'); (() => { diff --git a/yarn.lock b/yarn.lock index e16cd9c3673..90ba39a3251 100644 --- a/yarn.lock +++ b/yarn.lock @@ -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"