c9 / architect

A simple yet powerful plugin system for large-scale node applications
MIT License
982 stars 129 forks source link

Use of assertions #53

Open stavros-zavrakas opened 8 years ago

stavros-zavrakas commented 8 years ago

Hi there,

I am using the module but I am a bit confused how the assertions are used. As I see in the core project, you are using the assertions against the options object. For example, in the c9.core/client.js it is used like that:

assert(options.baseUrl, "Option 'baseUrl' is required") 

It is not used against imports though. For example, in the c9.vfs.standalone/standalone.js the preview handler it is imported like that:

var previewHandler = imports["preview.handler"];

What if a developer forget to add this dependency in the consume array? The previewHandler it will be undefined and it will break at runtime in this specific example. Why there is not an assertion like that:

assert(imports["preview.handler"], "preview.handler is required");

Is there something that I am missing?

nightwing commented 8 years ago

Adding assertions like that for every used service would make code too verbose/unreadable. requirement to manually update the list of dependencies in the consumes array is indeed problem, we should either make a eslint rule for it, or automatically infer list of dependencies using toString similar to what require.js does, but in the latter case performance overhead is significant for a large project like cloud9

stavros-zavrakas commented 8 years ago

We start adding assertions and finally the code became too verbose. Finally we god a decision to check the consumes array into the unit tests. The array is exported and we can compare the result with the definition in the unit test. If someone for a reason changes the dependencies (adding or removing a dependency) the unit test will break.

By the way, I saw that you are extending the chai expect library with the setupArchitectTest function (https://github.com/c9/core/blob/b24bf63f9f5a43a9647ab5cd20bee4c3e14d55fa/plugins/c9.vfs.standalone/www/test.js#L543).

Can this abstraction be helpful to check the consume dependencies or in general testing an architect application?

Many thanks!