-
Notifications
You must be signed in to change notification settings - Fork 36
The CommonJs Vs NamespacedJs Impedance Mismatch And Strawman
As of BladeRunnerJS 0.6, it's currently impossible to to have both CommonJs classes that fully utilize NamespacedJs classes, and NamespacedJs classes that fully utilize CommonJs classes. If we order the JavaScript as follows (which we currently do):
- CommonJs Classes
- CommonJs Globalization Code
- NamespacedJs Classes
Then NamespacedJs classes can make full use of CommonJs classes, but CommonJs classes can only make use of, but not extend or implement, NamespacedJs classes. This is because the CommonJs classes get defined within the globalization block, before the NamespacedJs classes have been defined. It is worth also pointing out that, even here, it's only possible for CommonJs classes to use NamespacedJs classes if they delay all require()
invocations for non static dependencies until the first time a class is constructed. For example:
var br = require('br/Core');
var SomeInterface = require('br/SomeInterface');
var Util;
var MyClass = function() {
// we don't require non-static dependencies until the first time a class is constructed
if(!Util) {
Util = require('br/Util');
}
};
br.implement(MyClass, SomeInterface);
MyClass.prototype.someMethod = function() {
Util.doStuff();
};
If we arrange the code like this instead:
- CommonJs Classes
- NamespacedJs Classes
- Globalization of CommonJs Classes
Then the CommonJs classes can theoretically make full use of the NamespacedJs classes, but the NamespacedJs classes can make no use whatsoever of the CommonJs classes, as they have neither been created nor globalized. Ultimately this is because CommonJs is viral, in that it requires all code be converted to CommonJs modules once any CommonJs modules are used.
A NamespacedJs class that looks like this:
pkg.MyClass = function() {
this.logger = new br.Logger();
};
br.Core.implement(pkg.myClass, br.SomeInterface);
could be automatically wrapped in a define module, where the transpiled class looked like this:
define('pkg/MyClass', function(require, exports, module) { requireAll(['br/Core', 'br/SomeInterface']);
pkg.MyClass = function() {
this.logger = new br.Logger();
};
br.Core.implement(pkg.myClass, br.SomeInterface);
module.exports = pkg.MyClass;
};
Here, only the static dependencies are required. This however is not a problem since all of the NamespacedJs modules will need to be explicitly globalized so they are available within the index page, and so that fully qualified class names within config files can still be resolved. Although a NamespacedJs source module (NS1
) may have a static dependency on a CommonJs source module (C1
), which itself has a static dependency on another NamespacedJs source module (NS2
), this isn't a problem since all of the dependencies that NS2
needs to define itself will be required in-line before the definition occurs.
The new requireAll()
method introduced above can be defined globally, rather than being provided separately for each source module, since it does not need to support relative require paths.
Currently, in BladeRunnerJS 0.6, static dependencies are defined using a hard-coded regular expression that looks for invocations of any of the following methods:
br.Core.extend()
br.Core.implement()
caplin.extend()
caplin.implement()
caplin.inherit()
This approach has always been limiting, but has recently become a sticking point given our decision to make 'br/Core' a class, rather than a thirdparty library. Although the thing being extended is brought in as a static dependency, we don't also create a static dependency on 'br/Core' itself, and we certainly don't do this for external classes or modules like 'caplin/extend', etc.
To overcome this problem, we ideally need to be able to detect which dependencies are used statically outside of any of the method blocks, and ideally without the computational expense of using a full parser. One approach to this that should be both performant and reliable would be to create a filter chain like this:
-
JsCommentStripper
->StringStripper
->CodeBlockStripper
or alternatively this:
-
StringStripper
->JsCommentStripper
->CodeBlockStripper
While JsCommentStripper
is a pre-existing class that is already known to be reliable, StringStripper
and CodeBlockStripper
would be new classes. The CodeBlockStripper
would be a very simple class that counts opening and closing curly braces, so it always knows how many levels of indentation it is within, where it only outputs code in the global scope. This naïve implementation can only work reliably if the code it given to process has first been stripped of comments and strings, since these might contain curly braces that shouldn't be counted, hence the need for the JsCommentStripper
and the StringStripper
.
To determine the complete list of dependencies we would run the trie over the class after it has been stripped of comments, whereas to determine the list of static dependencies we would run the trie over the class after it has been through the code-block stripper.
Because NamespacedJs classes will continue to access all exported variables globally, rather than using what is returned by require()
, the requireAll()
method can be designed to be tolerant of circular dependencies, meaning that our filter chain should be adequate rather than using a full parser, even in the face of the occasional mistake.
For example, in this class:
define('pkg/MyClass', function(require, exports, module) { requireAll(['pkg/Class1']);
pkg.Class2 = function() {
this.regExp = /[}%£]*/;
this.obj = new pkg.Class1();
};
module.exports = pkg.MyClass2;
};
'pkg/Class1'
has incorrectly been defined as a static dependency because of the curly brace within the literal regular expression. Assuming that 'pkg/Class1'
itself depends on 'pkg/Class2'
, and has caused it to be defined for the first time, then we would end up with a circular dependency when processing the defines.
Whereas the require()
method should fail-fast if it asked for a module that is currently in the process of being defined (a catastrophic circular dependency), the requireAll()
method can simply log a warning, but otherwise ignore subsequent requests for a module that is still being constructed. In most cases, programs with incorrectly determined static dependencies will still continue to work, and the developer will still get a good error message for cases where a modules that needs to exist has not yet been created, plus a warning in the logs showing the ultimate source of the problem.
Depending on how we order our filter chain, there will always be some bugs compared to using a proper JavaScript parser.
If we order our filter chain like this:
-
JsCommentStripper
->StringStripper
->CodeBlockStripper
then we will have these potential bugs:
Comment Stripper (only a problem if in global scope):
-
"//"
(lose dependency at the end of the line) (high probability) -
"/*"
(lose all dependencies after this appears) (medium probability)
String Stripper (only a problem if in global scope):
-
/["]*/
(lose dependency at the end of the line) (medium probability)
Code-Block Stripper:
-
/[}]*/
(gain dependencies in non global scope) (medium probability) -
/[{]*/
(lose dependencies in non global scope) (medium probability)
Whereas if we order our filter chain like this:
-
StringStripper
->JsCommentStripper
->CodeBlockStripper
then we will have these potential bugs:
String Stripper (only a problem if in global scope):
-
/* ' */
(lose dependency at the end of the line) (very low probability)
Comment Stripper (only a problem if in global scope):
-
/[/*]/
(lose dependencies to the end of the file) (low probability)
Code-Block Stripper:
-
/[}]*/
(gain dependencies in non global scope) -
/[{]*/
(lose dependencies in non global scope)
Stripping strings first is therefore the best approach.