WIP: #399 - organize vue codebase #408

sandrina-p · 2018-05-19T14:17:27Z

Closes #399.

So @taoeffect and @hubudibu this MR only moves files around and avoid any refactor inside the files itself, besides paths.

`/views`

Imports on router are now sorted Alphabetical for a easier comparation to views folder.
views folder has only the files imported at js/router.js. The rest was moved to its respective folder component or container
Despite CreateGroupSteps being imported at js/router.js, I've kept it at /components because I believe that's something we'll iterate during Build out and polish generic voting / proposals system #343 Voting System implementation.
Delete included.ejs because it's not used anywhere.

`/containers`

Files connected to store that aren't /views were moved into here.

`/components`

Rename the files that weren't [PascalCase].vue
Rename YourGroupsList to only GroupsList

`/js` and `/utils`

I've noticed some inconsistence inside /js. A little of everything... So, I've moved out from there what I consider utilities to a new folder /utils such as:
- currencies.js
- filters/toPercent.js - On its core a filter on vue is as simple as a utility, right? That's why I've moved. If you think we should keep filters folder, I can revert it easily.
js/utils.js has 2 functions lazyLoadVue and mapValues. I've split them in 2 files. Note that mapValues isn't being used at all at the moment. The comments talk about having our own lodash, so I've moved mapValues to a new file utils/giLodash.js and we can feed it when needed.

I didn't touch anything related to vuex, since I'm not sure the best approach. If you guys think the change is quick, I can do it on this MR too, otherwise, it can be done in another MR.

`linters`

Similar to !407, I've added a simple .editorconfig, just to standardize the 2 spaces, so a person doesn't need to change its editor to match the standard configs

taoeffect

I have not finished reviewing this PR, but I did add two comments, and I'm just pointing out that Travis CI failed, so double-check that everything works locally by running grunt test before submitting a PR.

If you did do that, then that's something else that needs to be fixed. grunt test locally should always match the result shown by Travis, and if it doesn't it means there's a test-related bug that needs to be fixed.

taoeffect · 2018-05-19T21:14:10Z

frontend/simple/utils/lazyLoadVue.js

+      .then(() => resolve(window[component]))
+      .catch((err) => reject(err))
+  }
+}


This file doesn't make sense to me. It seems to be only about one thing, a single function, and yet it does two things (setup vue-script2 as well).

In summary, two issues:

I think it is a bad idea to start doing 1-file-per-function.

This file is poorly named as what it does / what it's for isn't adequately described by the filename

So, I can keep it as it was before, lazyLoadVue and mapValues on the same file, and we can see later how to organize it better on another PR.

Regarding travis failing, I was at the train with low wifi, so I believe the push didn't successed. But it's fixed now :)

taoeffect · 2018-05-19T21:15:35Z

.editorconfig

+# 2 space indentation
+[*.py]
+indent_style = space
+indent_size = 2


I think .editorconfig should be added to .gitignore, not added to the repo, for two reasons:

We do not use *.py files anywhere inside the project

We already have rules for indentation, and those are defined by standard linter

I don't really agree, because one thing is the linter rules, another thing is the editor config, those are two separated concerns. Without .editorconfig everytime I start coding I need to go to my editor config and change the size spacing to 2, instead of 4 (my default). With this file, I don't need to. Not sure if I'm the only one who needs to do it. If yes, then okay, I'll add it to .gitignore

taoeffect · 2018-05-24T00:15:37Z

I want to really think this through so that we don't have to do it again, and so I've spent some time thinking about the categories the files we have belong to.

I think it's possible to organize things in a simpler way. For example, it's not clear to me why @hubudibu's StepAssistant is now a utility similar to lazyLoadVue and mapValues. StepAssistant is a pretty significant chunk of code, and a very important mixin that can potentially do a lot of things, and might eventually be split out into its own repo. So I don't consider it at all similar to tiny single-function files like toPercent.js.

I'm also really not a fan of creating entire files for single functions, especially one-liners like toPercent. I think those should remain in filters.js.

I notice we have:

Files related to the display of state (view)
Files related to the management of state (model)
Files related to constructing the app architecture (controller)
And support files for the above should be organized in subfolders of the above.

This is the standard MVC design pattern.

However, we have additional folders that don't fit quite well into this.

If you look at what folders we have in frontend/simple/, you'll see that we also organize files by category. For example, we put all sass files in sass/, all js files in js, third-party code and assets into assets/, and localizations in locales/.

Thus, a category separate from MVC suggests itself, and highlights the nature of what MVC is itself:

MVC = application logic / code
Everything else (sass, localization files, images, fonts) = static resources

However, you'll note the assets folder also contains code (e.g. assets/vendor/primus.js, assets/vendor/sprint.js), which violates our clean mental division above. Even weirder, sprint.js is not included in app.js (it is intended to be dynamically loaded via vue-script2, but we don't use this file at all right now, and might never use it), but primus.js is.

Furthermore, the contents of the assets folder is copied entirely into dist/simple/ by Gruntfile.babel.js, resulting in a server-side directory structure of:

/simple/
/simple/css/
/simple/fonts/
/simple/images/
/simple/js/
/simple/locales/
/simple/vendor/
/simple/app.js
/simple/index.html

The /simple/js folder above only contains one file: UserGroupView.js, and this is done in order to demonstrate how views in frontend/simple/views can be excluded from the app.js bundle and instead loaded dynamically, as-needed.

Server-side structure

So, it's clear, there are two things we need to consider: what does the directory structure look like before the bundle is built, and what does it look like on the server after it is built.

It may be easier to work backwards, starting with post-build, i.e. the files in the dist/ folder.

Let's review the 3 major conceptual categories we've discovered so far:

MVC (code)
static resources (images, localization files, css)
third-party stuff (code and resources)

So perhaps our server-side directory structure can look like this:

# entrypoint
/simple/index.html
/simple/app.js

# dynamically loaded views
/simple/views/UserGroupView.js

# assets that were created by us
/simple/assets/css/
/simple/assets/images/
/simple/assets/locales/

# assets that were created by a third-party
/simple/assets/third-party/fonts/
/simple/assets/third-party/libs/     # <- sprint.js, and maybe primus.js go here

Here we see our 3 major categories are still visible and are visibly separate as well:

MVC:
- app.js
- views/UserGroupView.js
static resources
- assets/* - except assets/third-party
third-party stuff (code and resources)
- assets/third-party/*

From this we can deduce an improved developer-side structure as well.

Developer-side structure

Again, our 3 mental categories: MVC + static resources + third-party

Let's break them down individually, starting with MVC:

MVC

Using frontend/simple/ as our root, we have:

# entrypoint
index.html
main.js

# View - (displaying data - note 'containers' are just treated as components)
view/views/                  # e.g. Mailbox.vue
view/components/             # e.g. login-modal.vue, i18n.vue, TimeTravel.vue
view/helpers/filters.js      # Note they are *NOT* separated into a bunch of tiny files
view/helpers/StepAssistant.js
view/helpers/currencies.js
view/helpers/transitions.js
view/helpers/translations.js
view/helpers/validators.js    # ... etc ...

# Model - (loading / manipulating / saving data)
model/state.js
model/database.js
model/contracts/events.js
model/contracts/identity.js

# Controller - (setting things in motion)
controller/router.js         # lazyLoadVue could be defined here since it's only used here
controller/backend/index.js
controller/backend/hapi.js
controller/backend/interface.js

# Utilities that might be used in many places
utils/giLodash.js
utils/crypto.js

# Resources
assets/sass/
assets/images/
assets/locales/
assets/third-party/fonts/
assets/third-party/libs/

And, maybe main.js could be moved into controller/main.js, but that's a minor detail.

This is a kind of MVC + MVVM pattern, but conceptually it seems to organize well this way, at least it seems that way to me.

Conclusion

That, to me, seems to be a long-lasting and conceptually clear / well organized folder structure that should last for the life of the project.

As this does involve editing Gruntfile.babel.js a bit (search it for assets, locales, and sass), I understand if you'd prefer @hubudibu or myself do it.

Thoughts?

taoeffect · 2018-05-28T16:16:53Z

A suggestion during today's call is to modify the view/ folder to be like:

# View - (displaying data - note 'containers' are just treated as components)
view/Mailbox.vue         # etc. top-level views go here
view/components/         # views that get props only
view/containers/         # components that also access the vuex store

sandrina-p · 2018-05-31T09:31:41Z

@taoeffect and @hubudibu I think I've made it! 😄

With exactly 110 files updated, I had the attention to, at each folder/file move, do a commit for it and verify if still were still working.

Check frontend/simple new structure:

Some notes:

Instead of assets/third-party, I called it assets/vendor for being a more "standard" name.
I've kept lazyLoadVue.js in its own file to avoid adding noise to router.js, similar to controller/utils/pubsub.js
We now have 2 names for the same type of folder helpers and utils.
Ex: simple/utils/ and simple/views/helpers/. Se should stick with one name - I vote for utils just because is a smaller name.
utils/vuex-queue seems to not be used anywhere, should we kept the file? (it's still there for you to check)
Do care about declarations.js? I've noticed the file is pretty outdated and we aren't using flow as we should.

taoeffect · 2018-06-01T20:20:03Z

Do care about declarations.js? I've noticed the file is pretty outdated and we aren't using flow as we should.

Yes, this file is important for being able to run npm run flow to catch bugs.

utils/vuex-queue seems to not be used anywhere, should we kept the file?

Eh, we'll probably rewrite/delete it but for now just ignore it.

…azyLoadVue.js and utils/giLodash.js

…and js/events.js -> model/contracts

…used files

…js -> controller/

taoeffect · 2018-06-01T22:30:44Z

Great job!!

sandrina-p requested review from hubudibu and taoeffect and removed request for hubudibu May 19, 2018 14:20

sandrina-p force-pushed the chore/399-organize-vue-codebase branch from 3ec8980 to 7a59ab5 Compare May 19, 2018 16:20

taoeffect requested changes May 19, 2018

View reviewed changes

taoeffect mentioned this pull request May 19, 2018

#343 - Update Invitation View to match designs on #343 #407

Merged

sandrina-p force-pushed the chore/399-organize-vue-codebase branch from 7a59ab5 to ff8fd9f Compare May 20, 2018 10:37

sandrina-p force-pushed the chore/399-organize-vue-codebase branch 2 times, most recently from 503933f to ff8fd9f Compare May 26, 2018 09:14

sandrina-p changed the title ~~Chore/399 organize vue codebase~~ WIP: #399 - organize vue codebase May 29, 2018

sandrina-p force-pushed the chore/399-organize-vue-codebase branch 5 times, most recently from e49f073 to d467c1f Compare May 31, 2018 08:57

sandrina-p added 2 commits May 31, 2018 10:26

Move sass/ -> assets/

431bde4

Move locales/ -> assets/ and assets/fonts -> assets/vendor

deb5666

sandrina-p force-pushed the chore/399-organize-vue-codebase branch from d467c1f to 3e99a0d Compare May 31, 2018 09:31

sandrina-p force-pushed the chore/399-organize-vue-codebase branch from 3e99a0d to d1fa2c5 Compare May 31, 2018 09:38

sandrina-p added 7 commits June 1, 2018 21:29

Move js/crypto.js -> utils/. Split js/utils.js in controllers/utils/l…

eec7575

…azyLoadVue.js and utils/giLodash.js

Move js/backend/ -> controller/

23174f4

Move js/database.js and js/state.js -> model/ . Move js/indentity.js …

4f1b82f

…and js/events.js -> model/contracts

Move views/*.vue -> /views/views|containers|components/ and delete un…

9058cea

…used files

Move components/Group -> views/components/CreateGroupSteps

827cc1a

Move js/*.js -> views/utils && Join filters/*.js to util/filters.js

81bf013

Move js/vuex-queue.js and js/distribution -> utils/ | Move js/pubsub.…

e3ef021

…js -> controller/

sandrina-p added 2 commits June 1, 2018 21:32

Update .Gruntfile.babel.js with linter

bb30e26

Move views/views/*.vue -> views/*.vue

af2d845

sandrina-p force-pushed the chore/399-organize-vue-codebase branch 2 times, most recently from 412cbe1 to e021676 Compare June 1, 2018 20:59

Update dist/ to keep assets/** folder and exclude assets/sass from it

89ff5b4

sandrina-p force-pushed the chore/399-organize-vue-codebase branch 2 times, most recently from 43b02be to 4e434fd Compare June 1, 2018 22:24

Move assets/vendor/fonts -> assets/

a4d5d9a

sandrina-p force-pushed the chore/399-organize-vue-codebase branch from 4e434fd to a4d5d9a Compare June 1, 2018 22:25

taoeffect merged commit e9969e2 into okTurtles:master Jun 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: #399 - organize vue codebase #408

WIP: #399 - organize vue codebase #408

sandrina-p commented May 19, 2018

taoeffect left a comment

taoeffect May 19, 2018

sandrina-p May 20, 2018

sandrina-p May 20, 2018

taoeffect May 19, 2018

sandrina-p May 20, 2018

taoeffect commented May 24, 2018 •

edited

Loading

taoeffect commented May 28, 2018 •

edited

Loading

sandrina-p commented May 31, 2018 •

edited

Loading

taoeffect commented Jun 1, 2018

taoeffect commented Jun 1, 2018

WIP: #399 - organize vue codebase #408

WIP: #399 - organize vue codebase #408

Conversation

sandrina-p commented May 19, 2018

/views

/containers

/components

/js and /utils

linters

taoeffect left a comment

Choose a reason for hiding this comment

taoeffect May 19, 2018

Choose a reason for hiding this comment

sandrina-p May 20, 2018

Choose a reason for hiding this comment

sandrina-p May 20, 2018

Choose a reason for hiding this comment

taoeffect May 19, 2018

Choose a reason for hiding this comment

sandrina-p May 20, 2018

Choose a reason for hiding this comment

taoeffect commented May 24, 2018 • edited Loading

Server-side structure

Developer-side structure

MVC

Conclusion

taoeffect commented May 28, 2018 • edited Loading

sandrina-p commented May 31, 2018 • edited Loading

Some notes:

taoeffect commented Jun 1, 2018

taoeffect commented Jun 1, 2018

`/views`

`/containers`

`/components`

`/js` and `/utils`

`linters`

taoeffect commented May 24, 2018 •

edited

Loading

taoeffect commented May 28, 2018 •

edited

Loading

sandrina-p commented May 31, 2018 •

edited

Loading