From a65de96b29450ec53a02d2bc645672bff18c6d35 Mon Sep 17 00:00:00 2001
From: Brandon Labuschagne <blabuschagne@gitlab.com>
Date: Wed, 11 Nov 2020 17:45:44 +0000
Subject: [PATCH] Refactor based on review feedback
Place a general explaination at the top level
style guide and link to it from the vue and vuex
guides.
---
doc/development/fe_guide/style/vue.md | 60 +++++++++++++++++++++++++++
doc/development/fe_guide/vue.md | 29 +++++++------
doc/development/fe_guide/vuex.md | 4 +-
3 files changed, 78 insertions(+), 15 deletions(-)
diff --git a/doc/development/fe_guide/style/vue.md b/doc/development/fe_guide/style/vue.md
index 4588117f51d06..6b6ca9a6c71ea 100644
--- a/doc/development/fe_guide/style/vue.md
+++ b/doc/development/fe_guide/style/vue.md
@@ -57,6 +57,66 @@ Please check this [rules](https://github.com/vuejs/eslint-plugin-vue#bulb-rules)
1. Use `.vue` for Vue templates. Do not use `%template` in HAML.
+1. Explicitly define data being passed into the Vue app
+
+ ```javascript
+ // bad
+ return new Vue({
+ el: '#element',
+ components: {
+ componentName
+ },
+ provide: {
+ ...someDataset
+ },
+ props: {
+ ...anotherDataset
+ },
+ render: createElement => createElement('component-name'),
+ }));
+
+ // good
+ const { foobar, barfoo } = someDataset;
+ const { foo, bar } = anotherDataset;
+
+ return new Vue({
+ el: '#element',
+ components: {
+ componentName
+ },
+ provide: {
+ foobar,
+ barfoo
+ },
+ props: {
+ foo,
+ bar
+ },
+ render: createElement => createElement('component-name'),
+ }));
+ ```
+
+ We discourage the use of the spread operator in this specific case in
+ order to keep our codebase explicit, discoverable, and searchable.
+ This applies in any place where we'll benefit from the above, such as
+ when [initializing Vuex state](../vuex.md#why-not-just-spread-the-initial-state).
+ The pattern above also enables us to easily parse non scalar values during
+ instantiation.
+
+ ```javascript
+ return new Vue({
+ el: '#element',
+ components: {
+ componentName
+ },
+ props: {
+ foo,
+ bar: parseBoolean(bar)
+ },
+ render: createElement => createElement('component-name'),
+ }));
+ ```
+
## Naming
1. **Extensions**: Use `.vue` extension for Vue components. Do not use `.js` as file extension ([#34371](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/34371)).
diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md
index b90566bc2fcd4..578c364052e45 100644
--- a/doc/development/fe_guide/vue.md
+++ b/doc/development/fe_guide/vue.md
@@ -66,29 +66,32 @@ You should only do this while initializing the application, because the mounted
The advantage of providing data from the DOM to the Vue instance through `props` in the `render` function
instead of querying the DOM inside the main Vue component is avoiding the need to create a fixture or an HTML element in the unit test,
-which will make the tests easier. See the following example:
+which will make the tests easier.
+
+See the following example, also, please refer to our [Vue style guide](style/vue.md#basic-rules) for additional
+information on why we explicitly declare the data being passed into the Vue app;
```javascript
// haml
#js-vue-app{ data: { endpoint: 'foo' }}
// index.js
-document.addEventListener('DOMContentLoaded', () => new Vue({
- el: '#js-vue-app',
- data() {
- const dataset = this.$options.el.dataset;
- return {
- endpoint: dataset.endpoint,
- };
- },
+const el = document.getElementById('js-vue-app');
+
+if (!el) return false;
+
+const { endpoint } = el.dataset;
+
+return new Vue({
+ el,
render(createElement) {
return createElement('my-component', {
props: {
- endpoint: this.endpoint,
+ endpoint
},
});
},
-}));
+}
```
> When adding an `id` attribute to mount a Vue application, please make sure this `id` is unique across the codebase
@@ -100,7 +103,7 @@ By following this practice, we can avoid the need to mock the `gl` object, which
It should be done while initializing our Vue instance, and the data should be provided as `props` to the main component:
```javascript
-document.addEventListener('DOMContentLoaded', () => new Vue({
+return new Vue({
el: '.js-vue-app',
render(createElement) {
return createElement('my-component', {
@@ -109,7 +112,7 @@ document.addEventListener('DOMContentLoaded', () => new Vue({
},
});
},
-}));
+});
```
#### Accessing feature flags
diff --git a/doc/development/fe_guide/vuex.md b/doc/development/fe_guide/vuex.md
index fb64095bbaae1..7765ba04d40cf 100644
--- a/doc/development/fe_guide/vuex.md
+++ b/doc/development/fe_guide/vuex.md
@@ -360,8 +360,8 @@ export default initialState => ({
```
We've made the conscious decision to avoid this pattern to aid in the
-discoverability and searchability of our frontend codebase. The reasoning for
-this is described in [this
+discoverability and searchability of our frontend codebase. The same applies
+when [providing data to a Vue app](vue.md#providing-data-from-haml-to-javascript). The reasoning for this is described in [this
discussion](https://gitlab.com/gitlab-org/frontend/rfcs/-/issues/56#note_302514865):
> Consider a `someStateKey` is being used in the store state. You _may_ not be
--
GitLab