Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated dependencies, added eslint, rollup, plus some fixes #521

Merged
merged 11 commits into from
Feb 4, 2019

Conversation

taoeffect
Copy link
Member

@taoeffect taoeffect commented Jan 30, 2019

This PR

  • Removes the no-longer-maintained vueify and switches us from browserify to the more advanced (and more rigid) rollup
  • Removes all related dependencies to vueify and browserify
  • Updates many dependencies (including babel to version 7) and fixes critical npm audit warnings
  • Extracts component scss out of the bundle and into a component.css file
  • Adds support for dynamically lazy-loading components with import() and shows an example of how to do that with UserGroup.vue in router.js
  • Moved all files in frontend/simple up one directory and placed all router links under /app URL (Move the stuff in frontend/simple up one directory #526) in order to make it easier to serve the app using third-party tools
  • Significantly improves our linter by switching from standard to eslint with eslint-plugin-standard
  • Deleted all the old stuff in frontend/_static
  • Creates a new top-level folder called historical under which old but potentially useful code can be kept
  • Updates browserslist key in package.json so that async/await do not get transformed by babel

I recommend viewing the diff with whitespace changes turned off.

TODO

  • fix import of circle slider
  • Try Grunt integration with rollup.watch instead of rollup.rollup! Replace rollup-plugin-serve with serve, or better yet, just keep what we currently have with grunt and run rollup via grunt / grunt-exec that didn't work out too well. So now am thinking maybe I should try with browser-sync? Or maybe just switch to webpack...? Maybe leave it all for a future PR.
  • Remove all browserify related dependencies and migrate any remaining stuff to rollup (for example, by using script2ify with rollup-plugin-browserify-transform
  • See if it is better to replace Grunt with npm scripts or to stick with it
  • Use eslint with eslint-config-standard so that it lints .vue files (instead of using standard directly, because it doesn't support that for some reason)
  • Make the SASS setup coherenet, simple and DRY
  • Connect the SASS setup with the vue code so that JS has access to theme variables from the SASS
  • NOTE: this is now part of issue Fix eslint and web console errors and warnings, plus verify sass support in components #525 - Verify that (1) vueify is loading the sass that's imported in Sidebar.vue, and (2) that when I removed ../../ from // @import "../../node_modules/bulma/sass/utilities/initial-variables"; (and all the other ../../node_modules), that I didn't break anything. Re-add the ../../ if I did. Choose either the grunt-sass, or better yet (so that @import can be called from components), remove grunt-sass and use the bundler's sass plugin (whatever the equivalent for vueify/rollup/webpack is)
  • Create issues out of any of the above that need to be created, as well as the rollup log output below:
(!) VuePlugin plugin: <contribution v-for="contribution in fakeStore.receiving.nonMonetary">: component lists rendered with v-for should have explicit keys. See https://vuejs.org/guide/list.html#key for more info.
frontend/simple/views/Contributions.vue
(!) VuePlugin plugin: <contribution v-for="contribution in fakeStore.giving.nonMonetary">: component lists rendered with v-for should have explicit keys. See https://vuejs.org/guide/list.html#key for more info.
frontend/simple/views/Contributions.vue
(!) VuePlugin plugin: <voting-banner v-for="proposal in Object.values(proposals)">: component lists rendered with v-for should have explicit keys. See https://vuejs.org/guide/list.html#key for more info.
frontend/simple/views/containers/Proposals.vue
(!) VuePlugin plugin: <voting v-for="proposal in groupProposals.notVoted">: component lists rendered with v-for should have explicit keys. See https://vuejs.org/guide/list.html#key for more info.
frontend/simple/views/containers/Proposals.vue
(!) VuePlugin plugin: <voting v-for="proposal in groupProposals.own">: component lists rendered with v-for should have explicit keys. See https://vuejs.org/guide/list.html#key for more info.
frontend/simple/views/containers/Proposals.vue
(!) VuePlugin plugin: <voting v-for="proposal in groupProposals.alreadyVoted">: component lists rendered with v-for should have explicit keys. See https://vuejs.org/guide/list.html#key for more info.
frontend/simple/views/containers/Proposals.vue
(!) VuePlugin plugin: <list-item v-for="id in list.order">: component lists rendered with v-for should have explicit keys. See https://vuejs.org/guide/list.html#key for more info.
frontend/simple/views/components/Chatroom/ConversationsList.vue
(!) VuePlugin plugin: <menu-item v-for="group in groupsByName">: component lists rendered with v-for should have explicit keys. See https://vuejs.org/guide/list.html#key for more info.
frontend/simple/views/containers/sidebar/GroupsList.vue
(!) VuePlugin plugin: <message v-for="message in ephemeral.pendingMessages">: component lists rendered with v-for should have explicit keys. See https://vuejs.org/guide/list.html#key for more info.
frontend/simple/views/components/Chatroom/ChatMain.vue
(!) VuePlugin plugin: <avatar v-for="user in founders">: component lists rendered with v-for should have explicit keys. See https://vuejs.org/guide/list.html#key for more info.
frontend/simple/views/containers/Chatroom/ConversationGreetings.vue

- Removed no longer used dependencies
- Added fully support for code-splitting and lazy loading of components
  (see example with route `/user-group`)
- Replaced `standard` with `standard`-based `eslint` setup that catches
  a lot more
- Deleted `frontend/_static` folder (next is moving `frontend/simple`
  into `frontend`)
- Added `historical/` folder where potentially useful old code can be
  stored for future reference
- Using `rollup-plugin-css-only` to extract component stylesheets out
  of the app bundle and into a separate `component.css` file
- Updated a bunch of dependencies
@taoeffect taoeffect changed the title WIP: Updated dependencies, added rollup, kinda works, plus some fixes that it caught Updated dependencies, added rollup, kinda works, plus some fixes that it caught Feb 2, 2019
@taoeffect
Copy link
Member Author

OK @pieer this PR is ready for your review! 😄 🎉

Please also see the related issue that I just created: #525

@taoeffect taoeffect changed the title Updated dependencies, added rollup, kinda works, plus some fixes that it caught Updated dependencies, added rollup, plus some fixes that it caught Feb 2, 2019
@taoeffect taoeffect changed the title Updated dependencies, added rollup, plus some fixes that it caught Updated dependencies, added eslint, rollup, plus some fixes that it caught Feb 2, 2019
@taoeffect taoeffect changed the title Updated dependencies, added eslint, rollup, plus some fixes that it caught Updated dependencies, added eslint, rollup, plus some fixes Feb 2, 2019
@taoeffect taoeffect mentioned this pull request Feb 4, 2019
frontend/main.js Outdated
@@ -53,6 +53,8 @@ async function startApp () {
new Vue({
router: router,
components: {
// Sidebar: await import('./views/containers/sidebar/Sidebar.vue'),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented code?

@@ -78,21 +78,24 @@
.fade-leave-to /* .fade-leave-active below version 2.1.8 */ {
opacity: 0;
}
</style>
<style scoped>

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why this code is going to global css now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add the scoped back in, but also have added a bunch of comments about doing this properly with transitions.js (which currently has a poor implementation that isn't really being used), that's something that I hope you can improve.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #529 to address this.

Copy link
Collaborator

@pieer pieer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good. Great work!

@taoeffect taoeffect merged commit f44a15a into okTurtles:master Feb 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants