Closed stubailo closed 8 years ago
There's actually a mini-tutorial on refactoring in the Cookbook.
Refactoring Process
Refactoring Patterns
Refactoring MVC to Components
I think the right way to build a Meteor app is to structure your code around the features you're building, not the stack layers. This means no top-level client and server directories with all of the code inside one or the other; instead you have a folder called news-feed and another called profile and they contain the relevant code for each. This also puts you on a good path to factor those into modules or packages later.
Approaching application design around features is fine. No argument there.
But I would disagree with the conclusion that there's no need for top-level client and server directories. First of all, those top-level directories are where code gets placed before features are completed. They're the work-area where stuff gets mish-mashed before being solidified into a package proper. Second, they're part of a refactoring process. Third, and this is important, most of our components sort of wind up using the application scaffolding and reserve words anyhow!
So, I'd suggest that we actually make the /server
and /client
directories isomorphic, and add them to the package API somehow. Or at the very least, explain that the application scaffold is perfectly valid to placed within packages.
I agree that there is a need for generic client/server directories. Can I get some clarification on your points?
What would it look like to "make the /server and /client directories isomorphic"?
Also, what do you mean by "application scaffolding and reserve words"?
I just mean that the /client
, /server
, and /test
directories are ideal ways to organize packages. It's possible to get a component working by simply specifying the api.use('templating')
package; but if a dev is truly building a package around an entire feature, that generally means they're going to need mongo and livedata, which means adding api.use('meteor-platform')
instead to get both UI templating and database storage.
So the package.js format we're finding ourselves using over and over again looks something like this:
Package.onUse(function (api) {
// we're basically adding the whole stack in the package
api.versionsFrom('1.2.2');
api.use('meteor-platform@1.2.2');
api.use('less');
// add organizing files in the package in the same way we orgnaized them in the app
api.addFiles('lib/myComponent.js', ['client', 'server']);
api.addFiles('client/subscription.js', 'client');
api.addFiles('server/publication.js', 'server');
// example: add multiple files to a location using an array
api.addFiles([
'client/components/myComponent/myComponent.js',
'client/components/myComponent/myComponent.html',
'client/components/myComponent/myComponent.less'
], 'client');
});
So rather than keeping the package files in a flat directory, we're just using the default scaffold structure to organize files. Here's a couple of packages in the wild that are built around specific features. Note how they have a directory structure that looks like a mini-app. Which makes intuitive sense if these packages deliver a unit of app functionality.
https://github.com/awatson1978/clinical-collaborations-ui https://github.com/awatson1978/clinical-hipaa-audit-log
I guess what I'm trying to say is that the reserved directories are associated with deployment technologies (server, client, test runner, etc); and should be consistently recognized in both apps and packages.
@awatson1978 that's an interesting idea! I guess it makes the package code more browsable so that you can see which files are for what without reading package.js
. I think that's great.
@stubailo We're using the same approach for exactly this reason. Especially if you work with multiple devs it makes things a lot more clear and can help avoid security issues (avoids putting secret stuff in client-side files).
So basically we split up features into packages and packages into a client, server and shared folder. Works great so far.
Here's some initial work on this! https://github.com/meteor/guide/blob/master/content/structure.md
@stubailo have you checked out https://meteor.hackpad.com/Building-Large-Apps-Tips-d8PQ848nLyE ?
And I agree about having separate apps for your admin, front-end, etc. for larger apps. It makes sense from a security point of view, and also from a performance point of view (at least until Meteor implements conditional loading). I haven't done it for Telescope because I want to keep it easy to deploy, but in an ideal situation I would consider it.
@SachaG nice article!
Some of it is general non-meteor-specific engineering advice (you'll spend most of your time changing existing code, etc).
Some things I could add based on the article, with some discussion:
MyNamespace.X = something
anywhere in any package, and it can be super hard to track down where it happened.app-core
seems like a pretty good idea. I was thinking of suggesting that you could put that code in the actual app, outside of the packages. I wonder what the pros and cons to this are.packagecheck
- that's a pretty useful tool. I think I should mention it for sure.package.js
file individually every time you update them (whereas in Telescope for example I can make every package depend on the new telescope:core
with a single find/replace). Also, so far I haven't ran into any downside with packages having extra unneeded dependencies. telescope:email
package adds it methods to Telescope.email
, not Telescope
. I feel like that's a good solution?app-core
code in your app's code, then you can't have any package code that depends on it, since it'll be loaded last. So I prefer having everything in packages and making sure I can control load order. Hey @stubailo
I'd like to share our application structure with you in case it triggers any ideas.
We use a hybrid approach and start with a core package that creates a global namespace. It looks like this:
/**
* This is the only file where globals should be declared.
* We do this, so we can have all other files in strict mode.
*/
/* globals Simian: true */
Simian = {
Constants: {},
// Small reusable utilities
Utils: {},
// Simple schemas for our models
Schemas: {},
// Meteor Mongo Collections
Collections: {},
Views: {},
// Domain specific logic and state on the client side.
Stores: {},
// Domain specific logic.
Services: {},
Subscriptions: {},
// Reusable components.
Components: {},
// Client-side events.
Actions: {},
// Server APIs in form of Meteor methods.
Api: {}
};
var global = this;
global.Simian = Simian;
Both the client and server get this file so this namespace is isomorphic.
This structure allows us to easily isolate code for testing and make it easy to know which code should go under which namespace. It also makes Webstorm (and other smart IDE's) autocomplete work well.
We have a Simian.Application class that configures the application. It creates an dependency injector and maps and instantiates all the parts of the app.
Then we have a features directory that contains client
and server
directories, where we extend the global name space at the correct point:
...
├── features
│ ├── editor
│ │ ├── client
│ │ └── server
│ ├── workspace
│ │ ├── client
│ │ └── server
│ └── ...
...
The code under the features folders is very specific to the feature being coded. Here's some sample code from the editor feature above that shows how we place classes on the global namespace:
class EditorService {
constructor() {
}
getEditor() {
return this.editor.get();
}
}
Simian.Services.Editor = EditorService;
Then we place all cross-cutting client concerns in the /client
or /server
folders like this:
...
├── client
│ ├── components
│ ├── layout
│ ├── lib
│ └── styles
├── server
│ └── publications
...
The client
and server
folders contain code that is very specific to the app. Most common case is package configuration for the app.
When the code is not that app specific and can be reused later in another app, then we put it in a package.
Our testing structure follows the same approach and looks like this:
tests
├── cucumber
│ └── features
│ ├── editor
│ ├── workspace
│ ├── _support
│ │ └── _widgets
│ └── step_definitions
└── jasmine
├── client
│ └── unit
│ ├── features
│ │ ├── editor
│ │ └── workspace
│ ├── lib
│ └── quarantine
└── server
└── integration
├── features
│ │ ├── editor
│ │ └── workspace
├── lib
└── quarantine
Finally, we can't wait for ES6 module support!
Hope the above helps.
Interesting - what are the testing benefits of having a single namespace? Mocking?
Isolation. Being able to access units of code means we don't need to load the entire meteor context when testing = ultra fast unit tests.
So yes, mocking!
Sorry, one more thing: Isolation gives us the ability to do mocking, but also other test doubles like stubs. We can also inject a this
context by using apply
I just mean like why is attaching to one namespace better than having a separate named export per package?
Oh. All the code that is app specific lives under the one export as not to pollute the global namespace
Packages however that are not app specific would have their own named export
@SachaG, @samhatoum: if you attach the exports of every package to a global namespace, do you still declare dependencies between packages for load order purposes?
I'm slightly concerned that by attaching stuff to a global namespace it becomes easy to accidentally forget to depend on a package but use it's export anyway. We actually had this problem in Meteor for a while because you didn't need to depend on mongo
to get Meteor.Collection
, as long as that was depended on by someone else.
Maybe this is a problem for published packages, but not for an app?
@samhatoum one more question:
We do this, so we can have all other files in strict mode.
How does having a single place that defines globals enable the rest of your app to be in strict mode? You mean because you never need to do X = Y
without a var
statement? Presumably, that part of the code could also do Simian.X = Y
and still be strict?
@SachaG (sorry for spamming with comments :P) is there any other reason to split your project into multiple apps other than having separate UIs? Is there something about data services or similar that could be useful to mention?
do you still declare dependencies between packages for load order purposes
We only declare the dependency on the core in the main app. We only ever write packages if they are independent of the app and don't need the namespace at all.
Presumably, that part of the code could also do Simian.X = Y and still be strict?
Exactly
@SachaG (sorry for spamming with comments :P) is there any other reason to split your project into multiple apps other than having separate UIs? Is there something about data services or similar that could be useful to mention?
I'm not sure, as I haven't actually done it myself. But speaking of multiple apps, how to share user accounts and login tokens between different apps is a question that comes around pretty frequently about Telescope.
In the project I'm working for we have multiple apps. The reason is having multiple UIs and I can't think of any other reason. However, there are a ton of reasons why you would want multiple UIs.
We have some extra build steps where the client and server program is taken out of the tarball from meteor build. The front-end gets some extra processing (css autoprefixing and the shared part of the css and js is stripped off and served by a cdn). The remaining of the front-end is served by nginx and the back-end runs in multiple docker containers. In front of this we have HAproxy. Because of this, there is a single point of entry (single domain), so sharing login tokens is a non-issue.
Wow, that sounds very advanced - do you have an article or post about this strategy I could link to?
We use a hybrid approach and start with a core package that creates a global namespace.
I have one client that uses this approach also. It keeps them organized, and they get a lot of mileage out of it. Namespacing is good.
We have some extra build steps where the client and server program is taken out of the tarball from meteor build. The front-end gets some extra processing (css autoprefixing and the shared part of the css and js is stripped off and served by a cdn). The remaining of the front-end is served by nginx and the back-end runs in multiple docker containers. In front of this we have HAproxy. Because of this, there is a single point of entry (single domain), so sharing login tokens is a non-issue.
Yup. I've had two different clients which independently architected basically the same approach. But this may be increasingly less of an issue with the new AccountsServer
and AccountsClient
APIs. The nginx
and HAproxy
folks will still love themselves their proxy servers though. Some additional links: Working with Proxies
is there any other reason to split your project into multiple apps other than having separate UIs? Is there something about data services or similar that could be useful to mention?
Gotta talk about scale-out, yo. One of the main reasons for breaking an application up into smaller apps is so that we can implement scaling tiers. Having a year-end sale? Scale up the landing pages and shopping cart applets. Having a mid-year contest? Scale up the newsletter registration applet. Have a short-term contract processes data for a client? Scale up the interface and back-end worker instances.
Wow, that sounds very advanced - do you have an article or post about this strategy I could link to?
Well, it probably sounds more advanced than it is. We have some information in the companies internal wiki which I guess I could transform into something public if there is interest.
Actually, we first completely reimplemented the meteor build process based on gulp, but I guess we were too optimistic ("How much work can it be?") and when it was almost finished (in fact it worked), we decided to abandon it since it would be too much of a burden to maintain and it turned out to be a huge waste of time.
increasingly less of an issue with the new AccountsServer and AccountsClient APIs. The nginx and HAproxy folks will still love themselves their proxy servers though.
Yeah, for us the proxy will remain necessary. the main reason for the proxy is not SSO. Most of our clients are universities and most of them use Shibboleth (a SAML2 implementation), so that's kind of solved for us. The problem is that we have hardware boxes that decode and composite videos on-premise and they have to talk to our meteor cloud-app. These universities tend to have very strict rules for their firewalls, so that's the main reason for our proxy.
For us the reasons to split up the app into multiple apps are:
Yesterday I talked to @tmeasday about naming, which would probably be part of this article in a section about code style. Here's what we came up with. These are relatively arbitrary decisions based on consistency and what "makes sense" to us. I think having a prescribed consistent style trumps disagreements on the individual issues.
FuzzyPinkElephants = new Mongo.Collection('FuzzyPinkElephants');
simple:rest
to make them available over HTTPMeteor.methods({
"/fuzzy-pink-elephants/add-friends"() { ... }
});
As mentioned in the #29 (forms/methods) discussion, we'd like to wrap up methods so that you call them via a module, so these names might end up being more of an implementation detail.
We're aware that currently not all Meteor core packages follow this pattern, but they probably should.
It would make sense for templates to be namespaced via dots, like so:
<template name="fuzzyPinkElephants.thumbnailGrid">
...
</template>
It is probably worth it to make accessing these in JS simpler:
// Before
Template['fuzzyPinkElephants.thumbnailGrid'].helpers({ ... });
// After?
Template.fuzzyPinkElephants.thumbnailGrid.helpers({ ... });
I imagine this would be a pretty small change to the templating
package.
In the future, I hope templates are packaged as ES2015 modules and the whole namespacing question can disappear forever.
Template.x.helperName = f
/collection-name/insert
. They will be taking up valuable real estate, and probably shouldn't be there if you aren't using allow/deny.@stubailo A remark about:
The identifiers of methods and publications should look like URLs: slash-separated, kebab-case The first part of the slash is the relevant collection name or module
I've been doing this in my own projects and I've found that using the collection name as first part is troublesome, since methods and publications can be about multiple collections at once. I've found that for methods you can usually indicate what is the most relevant collection, but I have lots of publications that return an array of cursors where there is no real master collection. So oftentimes, using the collection name is weird and seems like you're trying to simulate REST too much. Furthermore, using the module name makes it easier to find where the publication is defined (which is often hard to keep track of in larger applications with lots of packages)
This all looks pretty standard.
One question: the idea that packages only export a single item seems to conflict with the goal of feature-oriented packages, no? Example: an accounts entry package might export a server side object for authentication, and client-side templates that need to be attached to application code via spacebar helpers. Exporting only a single item seems like a good idea in theory, but does the architecture actually support it? I feel like in practice it's often necessary to export multiple things.
Also, no mention of CSS or HTML naming conventions and namespacing. Typically, one wants to have an html DOM structure and style class names which explicitly match the template name. So having a div that defines the boundary of the template like this:
<template name="fuzzyPinkElephants">
<div class="fuzzyPinkElephants">
<div class="thumbnailGrid">
</div>
</div>
</template>
Can then be used with a CSS preprocessor, like this. (LESS, Stylus, and SCSS all support this style syntax).
.fuzzyPinkElephants{
.thumbnailGrid{
}
}
Which compiles down to the following:
.fuzzyPinkElephants .thumbnailGrid {
}
The end result is nicely encapsulated and well-behaved CSS that behaves as if it's locally scoped to a template, and has a well managed namespace that matches the template/component structure.
The above is a Blaze pattern; but the naming convention and code pattern very much prepares templates to become webcomponents, blaze components, or react components.
@awatson1978, @sebakerckhof those are some good points!
About methods and collection names, I guess I'm just talking about the times where the module name is the same as the collection name (which seems to happen pretty often). For example, in Galaxy, we have a collection for apps, and the apps also have related collections like versions, activity feed, etc. I think in that case you can name your publication something like /apps/app-with-versions-and-feed
, so even though it would return different collections, it's still semantically related to the apps one.
Maybe what this means is we need a better-defined idea of a "module". In my mind it vaguely corresponds to some set of data, some API, some UI code, etc; is there some crisp definition we can come up with for this?
The intention of saying that each package/file exports a single item is so that it's easy to figure out where stuff is coming from. This concept definitely breaks down for templates since they are in a weird world of their own, but in the case where a package exports validation code, methods, and some utilities, would it not make sense to hang them off of one namespace?
I think it could make sense to make an explicit recommendation here to use a class that matches the template name. I don't want to go completely down the road of explaining how to write HTML and CSS, but this is a case where there is a meteor-specific convention that makes sense.
One issue for me is that as far as I can tell basically everyone uses kebab-case
for CSS classes. Would people find it weird to use camelCased
CSS classes? I'd probably convert my template name to kebab case for the class.
@stubailo - One minor mistake in your comment -- you wrote
Collection name in DB is lower case camel case
But then did a kebab-cased example.
Changed it to this:
FuzzyPinkElephants = new Mongo.Collection('FuzzyPinkElephants');
This could be slightly controversial but makes some sense to me in the name of consistency? It's hard to decide here but we should just pick something and run with it.
Yeah The convention has definitely been
FuzzyPinkElephants = new Mongo.Collection('fuzzyPinkElephants');
up until now. I've thought many times that your way is more consistent though.
One comment on snake_case vs camelCase vs ProperCase... some people use the different naming conventions to denote level of code maturity.
snake_case is used during initial prototyping; converted to camelCase after first code review; and only moved to ProperCase after test coverage, documentation, and formal namespace is added. This is a useful pattern for teams with different strength programmers; but requires that everybody understands that there's a refactor path from snake to camel to proper.
I would vote for ProperCase in the following:
FuzzyPinkElephants = new Mongo.Collection('FuzzyPinkElephants');
Because it's been baking for a long time, we're currently doing the documentation and namespacing discussions to move to ProperCase, and it does look better in Robomongo when the collections are in ProperCase.
The only downside is that people will confuse the DDP subscription name with the Cursor name. Guarantee you they will.
There are indeed many cases where a method or publication can be related to one collection. The problem is that there are also cases where it is not possible. Then you'll start mixing up module names and collection names. Say, I've got a collection named meetings
and a module named meetings
. So now a publication starting with /meetings
would be ambiguous....
I don't have a good definition for what a module is. In our case, it's usually a bunch of functionality grouped together. So we have a module that will initially define the meetings collections and certain functionality, but we have other modules that add features to the meeting system.
We also prepend it with a namespace that is a slug of our company name, so we have /companyslug/module/publication-name
. But that's just being overcautious to avoid any duplicates with third-party packages.
This concept definitely breaks down for templates since they are in a weird world of their own, but in the case where a package exports validation code, methods, and some utilities, would it not make sense to hang them off of one namespace?
That's certainly what we're trying to do. As long as you're aware that Templates are currently off in their own little world, and make it difficult to conform to the best practice; then yeah... it's all good.
One issue for me is that as far as I can tell basically everyone uses kebab-case for CSS classes. Would people find it weird to use camelCased CSS classes? I'd probably convert my template name to kebab case for the class.
Really? Coming from a C# background, the camelCase classes always seemed more natural to me, and requires less use of bracketed property accessors. Keep in mind that Javascript uses camelCase internally for CSS properties to avoid property accessors in multi-word properties.
document.getElementById(".fuzzyPinkElephants").style.backgroundColor = "#FF69B4";
Moving away from kabab-case to camelCase preps the code to be moved to JSS and style helpers, for advanced animation effects.
Not sure if it's a good idea, but in Telescope I've decided to use snake_case
for template names. This way the template name matches the file name, and it's also easy to search/replace for template names (especially if, say, you have the post_submit
template and the postSubmit
method).
I would also suggest a general template for package.js
. We use:
Package.describe({
name: 'core',
version: '0.1.0'
});
Package.onUse(function (api) {
api.versionsFrom('1.0.3.1');
// Core dependencies.
api.use([
]);
// 3rd party dependencies.
api.use([
]);
// Internal dependencies.
api.use([
]);
api.export(...);
api.addFiles([
]);
});
Because I think dots are the best for namespace in templates, I would suggest that the same is used also for methods and publish endpoints?
BTW, I think much better name than "publication" or "publish function" is "publish enpoint". I made some time ago a package called middleware with this idea. The idea is that there could be a package where you can define some extra metadata for your publish endpoints and then an automatic site could be generated for your publish API. Because that what it is. It would be great to be able to point developers to http://myapp.com/api/ (or wherever you put it in your router) and get a nice page with all public (no _
prefix) endpoints and methods.
I still haven't found a good patter how to split components, views, publish endpoints, and methods. Should that all be in one package/directory. Or should publish endpoints and methods be separate? Because often they can be reused. But often they are also very tight to a particular view (or even component).
I would also suggest that for app package we suggest that people do not use namespace:
. So it is then clear inside packages
directory, which packages are 3rd party packages (which you might have symlinked or git submoduled into it).
Packages export at most one thing
I think this is impossible. Sometimes you simply need more symbols.
I think this is impossible. Sometimes you simply need more symbols.
Do you have an example handy?
So currently in Meteor packages is that the author of the package defines the names of symbols. This is good because it makes you do not have to write at the beginning of every your file something like:
Foo = require('PackageNamespace').Foo;
Bar = require('PackageNamespace').Bar;
Baz = require('PackageNamespace').Baz;
The downside is that Foo
, Bar
and Baz
have to be relatively long to make them not be clashing with other stuff. So while you do not have to write PackageNamespace.Bar
everywhere you want to access Bar
, you probably should name the symbol PackageBar
at least or something.
Anyway, the point is that currently, when you cannot control imports in Meteor, then we can at least have the benefit of not having to write require
lines at the beginning. :-)
One concrete example, for Blaze Components I have BlazeComponent
and BlazeComponentDebug
symbols exposed. BlazeComponent
is what developers should extend for their components. Now imagine that BlazeComponentDebug
has to access some internal variables otherwise not exposed through BlazeComponent
. One option would be to have three packages, one "core" which exposes everything and then BlazeComponent
which most of the stuff, and BlazeComponentDebug
which exposes some other stuff. And putting debug into BlazeComponent.Debug
is not good because I do not want all components to have Debug
symbol. And using BlazeComponent.Base
as a base class is a bit long to type everytime you do class Foo extends BlazeComponent.Base
?
Hey @mitar.
I think our thinking was that you'd export BlazeComponent
and attach anything else you needed to that namespace.
I don't quite see how setting BlazeComponent.Debug = something
would mean every component would have that property? Can you explain?
Naively BlazeComponent.Debug
seems better than BlazeComponentDebug
in every possible way apart from that.
The aim of this convention is to work around the biggest [1] problem with the package system, which is that you can have symbols floating around the global namespace that you literally cannot tell where they've come from (if they are from atmosphere packages you can't even do a global search to find them).
[1] Don't know if it's the biggest, but it's very very real.
So, the issue is that the main use of the exported symbol for Blaze Components is to do something like:
class FooComponent extends BlazeComponent
Now, if there is BlazeComponent.Debug
that means, that there will be also FooComponent.Debug
inherited. Which you do not want.
Alternatively, we could make it so that you have to extend something like BlazeComponent.Base
:
class FooComponent extends BlazeComponent.Base
But that is then a lot of writing.
I think that if we want to promote this approach, we should allow somehow that you can "require" a subsymbol. Like you can do Future = require('fibers/future')
. So for example, how you would provide fibers in this one-symbol approach? I think it works in NPM because you can require sub-modules. so one can then provide debugging as a sub-module.
What does BlazeComponent.Debug
do though? If it affects FooComponent
it doesn't seem that weird that it would be on there. But maybe that's getting pedantic.
More concretely, don't you end up going new FooComponent
and using that? That thing won't have Debug
set on it.
(I say this because I'm imagining the practice of exporting a class as the top-level symbol of a package is really common).
I think that if we want to promote this approach, we should allow somehow that you can "require" a subsymbol.
If you're OK with that approach, how is that any better than:
const { Component } = BlazeComponents;
class Foo extends Component {
...
}
Compare to:
import Component from 'peerlibrary:blaze-components';
class Foo extends Component {
...
}
Actually, the first one is fewer characters.
Debug
has some debug utilities. They might need some internal code. But they do not really need the component itself. It is just a namespace thing (this is why I currently expose them as BlazeComponentsDebug
). So there is no need for that to be on FooComponent
. Also, Blaze Components classes have some static/class-level methods.
So this is probably not a problem, it is just polluting the components namespace. But because the whole point of this discussion is to keep namespaces clean, I am pointing out that by requiring one symbol, you are potentially dirtying some other namespace (a static/class level one). Not global, but still.
Article outline
(to be updated potentially for the Meteor 1.3 module world)
https://github.com/meteor/guide/blob/master/outlines/structure.md
Major decision points