Commit 3fac0ad8 authored by Mark Florian's avatar Mark Florian

Fix/ignore no-runtime-template-compiler violations

The existing violations of the @gitlab/no-runtime-template-compiler rule
fall into a few different categories:

- True violations
  - Explicit `template` options in Vue component definitions
  - Implicit violations by relying on the mounting element to contain
    the template, generated by the backend
- False violations
  - Instantiating a Vue SFC by spreading it into `new Vue`, e.g.:
    `new Vue({ el, ...SomeVueSFC });`
  - EE components which use `extends` of a CE component, which itself
    correctly uses an ahead-of-time compiled template
  - Pretty much any file that defines a global Vue mixin

The more straight-forward true violations have been fixed and verified
as working, although there doesn't seem to be existing unit tests for
them.

The aren't many examples of false violations in the codebase, though
it's feasible that the lint rule could be improved to not report these
in future.

The rule is disabled entirely for specs, since in that context, having
a runtime template compiler can be useful.

Addresses https://gitlab.com/gitlab-org/frontend/eslint-plugin/-/issues/24,
part of https://gitlab.com/groups/gitlab-org/-/epics/5142.
parent a8963416
...@@ -48,3 +48,4 @@ overrides: ...@@ -48,3 +48,4 @@ overrides:
- '**/spec/**/*' - '**/spec/**/*'
rules: rules:
"@gitlab/require-i18n-strings": off "@gitlab/require-i18n-strings": off
"@gitlab/no-runtime-template-compiler": off
/* eslint-disable no-new */ // This is a true violation of @gitlab/no-runtime-template-compiler, as it
// relies on app/views/shared/boards/components/_sidebar.html.haml for its
// template.
/* eslint-disable no-new, @gitlab/no-runtime-template-compiler */
import $ from 'jquery'; import $ from 'jquery';
import Vue from 'vue'; import Vue from 'vue';
......
<script>
import { GlIcon } from '@gitlab/ui';
import { __ } from '~/locale';
import { hide } from '~/tooltips';
export default {
components: {
GlIcon,
},
props: {
issueBoardsContentSelector: {
type: String,
required: true,
},
},
data() {
return {
isFullscreen: false,
};
},
methods: {
toggleFocusMode() {
hide(this.$refs.toggleFocusModeButton);
const issueBoardsContent = document.querySelector(this.issueBoardsContentSelector);
issueBoardsContent.classList.toggle('is-focused');
this.isFullscreen = !this.isFullscreen;
},
},
i18n: {
toggleFocusMode: __('Toggle focus mode'),
},
};
</script>
<template>
<div class="board-extra-actions">
<a
ref="toggleFocusModeButton"
href="#"
class="btn btn-default has-tooltip gl-ml-3 js-focus-mode-btn"
data-qa-selector="focus_mode_button"
role="button"
:aria-label="$options.i18n.toggleFocusMode"
:title="$options.i18n.toggleFocusMode"
@click="toggleFocusMode"
>
<gl-icon :name="isFullscreen ? 'minimize' : 'maximize'" />
</a>
</div>
</template>
import $ from 'jquery';
import Vue from 'vue'; import Vue from 'vue';
import { GlIcon } from '@gitlab/ui'; import ToggleFocus from './components/toggle_focus.vue';
import { hide } from '~/tooltips';
export default (ModalStore, boardsStore) => { export default () => {
const issueBoardsContent = document.querySelector('.content-wrapper > .js-focus-mode-board'); const issueBoardsContentSelector = '.content-wrapper > .js-focus-mode-board';
return new Vue({ return new Vue({
el: document.getElementById('js-toggle-focus-btn'), el: '#js-toggle-focus-btn',
components: { render(h) {
GlIcon, return h(ToggleFocus, {
props: {
issueBoardsContentSelector,
},
});
}, },
data: {
modal: ModalStore.store,
store: boardsStore.state,
isFullscreen: false,
},
methods: {
toggleFocusMode() {
const $el = $(this.$refs.toggleFocusModeButton);
hide($el);
issueBoardsContent.classList.toggle('is-focused');
this.isFullscreen = !this.isFullscreen;
},
},
template: `
<div class="board-extra-actions">
<a
href="#"
class="btn btn-default has-tooltip gl-ml-3 js-focus-mode-btn"
data-qa-selector="focus_mode_button"
role="button"
aria-label="Toggle focus mode"
title="Toggle focus mode"
ref="toggleFocusModeButton"
@click="toggleFocusMode">
<gl-icon :name="isFullscreen ? 'minimize' : 'maximize'" />
</a>
</div>
`,
}); });
}; };
// This is a true violation of @gitlab/no-runtime-template-compiler, as it
// relies on app/views/projects/cycle_analytics/show.html.haml for its
// template.
/* eslint-disable @gitlab/no-runtime-template-compiler */
import $ from 'jquery'; import $ from 'jquery';
import Vue from 'vue'; import Vue from 'vue';
import Cookies from 'js-cookie'; import Cookies from 'js-cookie';
......
...@@ -28,19 +28,18 @@ class RecentSearchesRoot { ...@@ -28,19 +28,18 @@ class RecentSearchesRoot {
const { state } = this.store; const { state } = this.store;
this.vm = new Vue({ this.vm = new Vue({
el: this.wrapperElement, el: this.wrapperElement,
components: {
RecentSearchesDropdownContent,
},
data() { data() {
return state; return state;
}, },
template: ` render(h) {
<recent-searches-dropdown-content return h(RecentSearchesDropdownContent, {
:items="recentSearches" props: {
:is-local-storage-available="isLocalStorageAvailable" items: this.recentSearches,
:allowed-keys="allowedKeys" isLocalStorageAvailable: this.isLocalStorageAvailable,
/> allowedKeys: this.allowedKeys,
`, },
});
},
}); });
} }
......
/* eslint-disable no-param-reassign */ // This is a true violation of @gitlab/no-runtime-template-compiler, as it relies on
// app/views/projects/merge_requests/conflicts/components/_diff_file_editor.html.haml
// for its template.
/* eslint-disable no-param-reassign, @gitlab/no-runtime-template-compiler */
import Vue from 'vue'; import Vue from 'vue';
import { debounce } from 'lodash'; import { debounce } from 'lodash';
......
/* eslint-disable no-param-reassign */ // This is a true violation of @gitlab/no-runtime-template-compiler, as it relies on
// app/views/projects/merge_requests/conflicts/components/_inline_conflict_lines.html.haml
// for its template.
/* eslint-disable no-param-reassign, @gitlab/no-runtime-template-compiler */
import Vue from 'vue'; import Vue from 'vue';
import actionsMixin from '../mixins/line_conflict_actions'; import actionsMixin from '../mixins/line_conflict_actions';
......
...@@ -15,6 +15,9 @@ import utilsMixin from '../mixins/line_conflict_utils'; ...@@ -15,6 +15,9 @@ import utilsMixin from '../mixins/line_conflict_utils';
required: true, required: true,
}, },
}, },
// This is a true violation of @gitlab/no-runtime-template-compiler, as it
// has a template string.
// eslint-disable-next-line @gitlab/no-runtime-template-compiler
template: ` template: `
<table class="diff-wrap-lines code js-syntax-highlight"> <table class="diff-wrap-lines code js-syntax-highlight">
<tr class="line_holder parallel" v-for="section in file.parallelLines"> <tr class="line_holder parallel" v-for="section in file.parallelLines">
......
// This is a true violation of @gitlab/no-runtime-template-compiler, as it
// relies on app/views/projects/merge_requests/conflicts/show.html.haml for its
// template.
/* eslint-disable @gitlab/no-runtime-template-compiler */
import $ from 'jquery'; import $ from 'jquery';
import Vue from 'vue'; import Vue from 'vue';
import FileIcon from '~/vue_shared/components/file_icon.vue'; import FileIcon from '~/vue_shared/components/file_icon.vue';
......
...@@ -26,7 +26,13 @@ export default (props = {}) => { ...@@ -26,7 +26,13 @@ export default (props = {}) => {
dashboardProps: { ...dataProps, ...props }, dashboardProps: { ...dataProps, ...props },
}; };
}, },
template: `<router-view :dashboardProps="dashboardProps"/>`, render(h) {
return h('RouterView', {
attrs: {
dashboardProps: this.dashboardProps,
},
});
},
}); });
} }
}; };
// This is a true violation of @gitlab/no-runtime-template-compiler, as it
// relies on app/views/admin/application_settings/_gitpod.html.haml for its
// template.
/* eslint-disable @gitlab/no-runtime-template-compiler */
import Vue from 'vue'; import Vue from 'vue';
import initUserInternalRegexPlaceholder from '../account_and_limits'; import initUserInternalRegexPlaceholder from '../account_and_limits';
import IntegrationHelpText from '~/vue_shared/components/integrations_help_text.vue'; import IntegrationHelpText from '~/vue_shared/components/integrations_help_text.vue';
......
...@@ -7,10 +7,13 @@ document.addEventListener('DOMContentLoaded', () => { ...@@ -7,10 +7,13 @@ document.addEventListener('DOMContentLoaded', () => {
remainingTimeElements.forEach( remainingTimeElements.forEach(
(el) => (el) =>
new Vue({ new Vue({
...GlCountdown,
el, el,
propsData: { render(h) {
endDateString: el.dateTime, return h(GlCountdown, {
props: {
endDateString: el.dateTime,
},
});
}, },
}), }),
); );
......
// This is a false violation of @gitlab/no-runtime-template-compiler, since it
// is simply defining a global Vue mixin.
/* eslint-disable @gitlab/no-runtime-template-compiler */
const ComponentPerformancePlugin = { const ComponentPerformancePlugin = {
install(Vue, options) { install(Vue, options) {
Vue.mixin({ Vue.mixin({
......
// This is a false violation of @gitlab/no-runtime-template-compiler, since it
// creates a new Vue instance by spreading a _valid_ Vue component definition
// into the Vue constructor.
/* eslint-disable @gitlab/no-runtime-template-compiler */
import Vue from 'vue'; import Vue from 'vue';
import MrWidgetOptions from 'ee_else_ce/vue_merge_request_widget/mr_widget_options.vue'; import MrWidgetOptions from 'ee_else_ce/vue_merge_request_widget/mr_widget_options.vue';
import VueApollo from 'vue-apollo'; import VueApollo from 'vue-apollo';
......
// This is a false violation of @gitlab/no-runtime-template-compiler, since it
// is simply defining a global Vue mixin.
/* eslint-disable @gitlab/no-runtime-template-compiler */
export default (Vue) => { export default (Vue) => {
Vue.mixin({ Vue.mixin({
provide: { provide: {
......
// This is a false violation of @gitlab/no-runtime-template-compiler, since it
// is simply defining a global Vue mixin.
/* eslint-disable @gitlab/no-runtime-template-compiler */
import { __, n__, s__, sprintf } from '../locale'; import { __, n__, s__, sprintf } from '../locale';
export default (Vue) => { export default (Vue) => {
......
<script> <script>
// This is a false violation of @gitlab/no-runtime-template-compiler, since it
// extends a valid Vue single file component.
/* eslint-disable @gitlab/no-runtime-template-compiler */
import BoardListHeaderFoss from '~/boards/components/board_list_header.vue'; import BoardListHeaderFoss from '~/boards/components/board_list_header.vue';
import { __, sprintf, s__ } from '~/locale'; import { __, sprintf, s__ } from '~/locale';
......
<script> <script>
// This is a false violation of @gitlab/no-runtime-template-compiler, since it
// extends a valid Vue single file component.
/* eslint-disable @gitlab/no-runtime-template-compiler */
import BoardListHeaderFoss from '~/boards/components/board_list_header_deprecated.vue'; import BoardListHeaderFoss from '~/boards/components/board_list_header_deprecated.vue';
import { __, sprintf, s__ } from '~/locale'; import { __, sprintf, s__ } from '~/locale';
import boardsStore from '~/boards/stores/boards_store'; import boardsStore from '~/boards/stores/boards_store';
......
<script>
import { GlButton, GlModalDirective, GlTooltipDirective } from '@gitlab/ui';
import { s__, __ } from '~/locale';
export default {
components: {
GlButton,
},
directives: {
GlTooltip: GlTooltipDirective,
GlModalDirective,
},
props: {
boardsStore: {
type: Object,
required: true,
},
canAdminList: {
type: Boolean,
required: true,
},
hasScope: {
type: Boolean,
required: true,
},
},
data() {
return {
state: this.boardsStore.state,
};
},
computed: {
buttonText() {
return this.canAdminList ? s__('Boards|Edit board') : s__('Boards|View scope');
},
tooltipTitle() {
return this.hasScope ? __("This board's scope is reduced") : '';
},
},
methods: {
showPage(page) {
return this.boardsStore.showPage(page);
},
},
};
</script>
<template>
<div class="gl-ml-3">
<gl-button
v-gl-modal-directive="'board-config-modal'"
v-gl-tooltip
:title="tooltipTitle"
:class="{ 'dot-highlight': hasScope }"
data-qa-selector="boards_config_button"
@click.prevent="showPage('edit')"
>
{{ buttonText }}
</gl-button>
</div>
</template>
};
import Vue from 'vue'; import Vue from 'vue';
import { GlButton, GlModalDirective, GlTooltipDirective } from '@gitlab/ui'; import ConfigToggle from './components/config_toggle.vue';
import { s__, __ } from '~/locale';
export default (boardsStore) => { export default (boardsStore) => {
const configEl = document.querySelector('.js-board-config'); const configEl = document.querySelector('.js-board-config');
...@@ -8,45 +7,15 @@ export default (boardsStore) => { ...@@ -8,45 +7,15 @@ export default (boardsStore) => {
if (configEl) { if (configEl) {
gl.boardConfigToggle = new Vue({ gl.boardConfigToggle = new Vue({
el: configEl, el: configEl,
components: { render(h) {
GlButton, return h(ConfigToggle, {
props: {
boardsStore,
canAdminList: configEl.hasAttribute('data-can-admin-list'),
hasScope: configEl.hasAttribute('data-has-scope'),
},
});
}, },
directives: {
GlTooltip: GlTooltipDirective,
GlModalDirective,
},
data() {
return {
canAdminList: this.$options.el.hasAttribute('data-can-admin-list'),
hasScope: this.$options.el.hasAttribute('data-has-scope'),
state: boardsStore.state,
};
},
computed: {
buttonText() {
return this.canAdminList ? s__('Boards|Edit board') : s__('Boards|View scope');
},
tooltipTitle() {
return this.hasScope ? __("This board's scope is reduced") : '';
},
},
methods: {
showPage: (page) => boardsStore.showPage(page),
},
template: `
<div class="gl-ml-3">
<gl-button
v-gl-modal-directive="'board-config-modal'"
v-gl-tooltip
:title="tooltipTitle"
:class="{ 'dot-highlight': hasScope }"
data-qa-selector="boards_config_button"
@click.prevent="showPage('edit')"
>
{{ buttonText }}
</gl-button>
</div>
`,
}); });
} }
}; };
...@@ -29934,6 +29934,9 @@ msgstr "" ...@@ -29934,6 +29934,9 @@ msgstr ""
msgid "Toggle emoji award" msgid "Toggle emoji award"
msgstr "" msgstr ""
msgid "Toggle focus mode"
msgstr ""
msgid "Toggle navigation" msgid "Toggle navigation"
msgstr "" msgstr ""
......
...@@ -15,7 +15,7 @@ module QA ...@@ -15,7 +15,7 @@ module QA
element :board_scope_modal element :board_scope_modal
end end
view 'ee/app/assets/javascripts/boards/config_toggle.js' do view 'ee/app/assets/javascripts/boards/components/config_toggle.vue' do
element :boards_config_button element :boards_config_button
end end
end end
......
...@@ -39,7 +39,7 @@ module QA ...@@ -39,7 +39,7 @@ module QA
element :boards_list element :boards_list
end end
view 'app/assets/javascripts/boards/toggle_focus.js' do view 'app/assets/javascripts/boards/components/toggle_focus.vue' do
element :focus_mode_button element :focus_mode_button
end end
......
import Vue from 'vue'; import { setHTMLFixture } from 'helpers/fixtures';
import RecentSearchesRoot from '~/filtered_search/recent_searches_root'; import RecentSearchesRoot from '~/filtered_search/recent_searches_root';
jest.mock('vue'); const containerId = 'test-container';
const dropdownElementId = 'test-dropdown-element';
describe('RecentSearchesRoot', () => { describe('RecentSearchesRoot', () => {
describe('render', () => { describe('render', () => {
let recentSearchesRoot; let recentSearchesRootMockInstance;
let data; let vm;
let template; let containerEl;
beforeEach(() => { beforeEach(() => {
recentSearchesRoot = { setHTMLFixture(`
<div id="${containerId}">
<div id="${dropdownElementId}"></div>
</div>
`);
containerEl = document.getElementById(containerId);
recentSearchesRootMockInstance = {
store: { store: {
state: 'state', state: {
recentSearches: ['foo', 'bar', 'qux'],
isLocalStorageAvailable: true,
allowedKeys: ['test'],
},
}, },
wrapperElement: document.getElementById(dropdownElementId),
}; };
Vue.mockImplementation((options) => { RecentSearchesRoot.prototype.render.call(recentSearchesRootMockInstance);
({ data, template } = options); vm = recentSearchesRootMockInstance.vm;
});
RecentSearchesRoot.prototype.render.call(recentSearchesRoot); return vm.$nextTick();
}); });
it('should instantiate Vue', () => { afterEach(() => {
expect(Vue).toHaveBeenCalled(); vm.$destroy();
expect(data()).toBe(recentSearchesRoot.store.state); });
expect(template).toContain(':is-local-storage-available="isLocalStorageAvailable"');
it('should render the recent searches', () => {
const { recentSearches } = recentSearchesRootMockInstance.store.state;
recentSearches.forEach((recentSearch) => {
expect(containerEl.textContent).toContain(recentSearch);
});
}); });
}); });
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment