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

Managing Dependencies #115

Closed
jgaehring opened this issue May 5, 2018 · 7 comments
Closed

Managing Dependencies #115

jgaehring opened this issue May 5, 2018 · 7 comments

Comments

@jgaehring
Copy link
Member

I encountered some issue when trying to update npm packages recently, and it's had me thinking more about the way we have dependencies split up across the client and native repos.

I wanted to upgrade some packages, as well as get rid of some dependencies in the native repo that should only really exist in the client repo, when I ran into a whole mess of headaches. Particularly, the vue-loading-spinner was giving me a lot of trouble when I tried to remove it, even though there are absolutely no parts of our code base still calling this component, neither in the client or native repos (it was repaced by vue-spinner-component. I don't know if this was because of some discrepancies in the lock files after I updated, or if it's how we've got Webpack configured, or some issue with the way I'm using npm link to use my local farmos-client repo as a dependency instead of the GitHub repo—or some combination of those three. But it's made me scrutinize the general architecture a little more.

Something that seems like a bit of a code smell to me is that the main Vue libraries (vue, vue-router, vuex, vue-loader, etc) are all installed in the native repo, yet most of the Vue components, the routes.js config file, and the main Vuex store are all contained in the client repo. For one thing, it will mean that if we want to switch out Vue from the client, we'll have to do the same on the native repo, and vice versa. But it also could make it hard to update Vue-specific dependencies in the future.

I'm still not sure what the best way to remedy this is, since the native repo is still supplying Vue plugins and mixins that need to be attached to the main Vue instance at runtime, and since Webpack needs to have a lot of control over the build process when it runs on the native repo. I'm not sure how it would effect that process if we moved the Vue libraries to the client repo. I also think I'll have a better feel for all this once I get more familiar with the Cordova build process. So I'm just leaving this hear for now to follow up on it later when I know more.

@mstenta
Copy link
Member

mstenta commented May 14, 2018

I don't have a full sense of the architecture, so take my thoughts with a grain of salt, but it seems that the "client" repo should be able to build

@mstenta mstenta closed this as completed May 14, 2018
@mstenta
Copy link
Member

mstenta commented May 14, 2018

Oops - ignore that - clicked wrong button. :-)

@mstenta mstenta reopened this May 14, 2018
@mstenta
Copy link
Member

mstenta commented May 14, 2018

I don't think I have enough understanding of the architecture to provide advice, but I'd be happy to look at it together sometime if that would help!

Ideally we should be able to build the client repo independently of the native repo, right? So it should include anything that it needs in it itself. Maybe we need another implementation of the client in order to see where the separations need to be? ie: offline module?

@jgaehring
Copy link
Member Author

Ideally we should be able to build the client repo independently of the native repo, right? So it should include anything that it needs in it itself.

Yea, I agree. I need to do a little deeper dive into how

Right now, I have the native repo making the function call that instantiates the main Vue object. I wanted to defer instantiation that way because it gave me the option to pass in the plugins as parameters, and then attach them just before instantiation actually occurred. At the time, I wasn't sure how else to do this, but there must be a better way. I'll just need dig deeper into the Vue docs.

Maybe we need another implementation of the client in order to see where the separations need to be? ie: offline module?

I thinking having another working implementation up and running will help in any number of ways, this issue included. I think there's a minimal level of separation I want to reach before trying that, such as just being able to cleanly run npm update to update our packages without stuff breaking, but yea, even prior to that it will be useful to think in terms of how do we build towards a separate implementation.

@jgaehring
Copy link
Member Author

jgaehring commented Jun 2, 2018

I spent a little while today working out the details of the dependency graph—ie, how it is right now, and how it ought to be—but ran up against a string of Webpack errors after trying to move npm packages around to where I want, remove dead packages, and update the ones that need patching. I was going to hold off reworking the Webpack config until after I got the dependencies worked out, but I'm thinking now it might be better to approach it from the opposite tack. If I separate the Webpack configurations into what's needed for Cordova, and what's needed for Vue, I should then be able to create clean Webpack files for both the native and client repos. That, I think, should silence the Webpack errors when I update npm packages. The client repo, currently, doesn't have any Webpack configuration. Instead it just defers to the native repo to build it; but that should definitely change if we're to treat it like a proper library. On that note, the conventional wisdom seems to favor Rollup for libraries, Webpack for deployment, so I may look at using that instead for the client, especially if I go the route of re-scaffolding the repo from scratch, which seems more and more like the easiest option.

At this point, my view is that the native repo should have all the Cordova and none of the Vue dependencies, and vice versa for the client repo. All the native repo does is take a JavaScript module (which could be made with Vue or React or just plain JS, for all it cares), and bundles it up with the data & login plugins (which do need to know about Vue, but I'm already starting to think of those as separate libraries at this point), then inserts a script tag pointing to that bundle within its own index.html* file. That's all Webpack needs to hand-off to Cordova, in the form of a /www directory, for Cordova to build it into the final Android and iOS packages.

* = That last point about the index.html file is also quite crucial, I realized, because that is what will make the client library truly portable. Along with the plugins it needs, it only needs to get passed a DOM selector, eg, #app or #container or just body, and then it can be used in any number of implementations and can be inserted anywhere in the DOM, either as the whole app, or just part of a larger app.

@jgaehring
Copy link
Member Author

jgaehring commented Jun 5, 2018

I'm going close this and break it up into the smaller issues it represents, as outlined below:

@mstenta mstenta transferred this issue from farmOS-legacy/farmOS-client Feb 19, 2019
@mstenta
Copy link
Member

mstenta commented Feb 19, 2019

(Transferring all issues from old repository. See #92)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants