iojs: introduce internal modules

Internal modules can be used to share private code between public modules without risk to expose private APIs to the user.
nodejs · vkurchatkin · Feb 14, 2015 · Mar 10, 2015 · Mar 10, 2015 · Mar 14, 2015
commit 0be359614aff0282381c753f490daa98bef8e1e2
diff --git a/lib/_http_common.js b/lib/_http_common.js
@@ -1,6 +1,6 @@
 'use strict';
 
-const FreeList = require('freelist').FreeList;
+const FreeList = require('internal/freelist').FreeList;
 const HTTPParser = process.binding('http_parser').HTTPParser;
 
 const incoming = require('_http_incoming');

diff --git a/lib/freelist.js b/lib/freelist.js
@@ -1,26 +1,3 @@
 'use strict';
 
-// This is a free list to avoid creating so many of the same object.
-exports.FreeList = function(name, max, constructor) {
-  this.name = name;
-  this.constructor = constructor;
-  this.max = max;
-  this.list = [];
-};
-
-
-exports.FreeList.prototype.alloc = function() {
-  //debug("alloc " + this.name + " " + this.list.length);
-  return this.list.length ? this.list.shift() :
-                            this.constructor.apply(this, arguments);
-};
-
-
-exports.FreeList.prototype.free = function(obj) {
-  //debug("free " + this.name + " " + this.list.length);
-  if (this.list.length < this.max) {
-    this.list.push(obj);
-    return true;
-  }
-  return false;
-};
+module.exports = require('internal/freelist');
diff --git a/lib/internal/freelist.js b/lib/internal/freelist.js
@@ -0,0 +1,26 @@
+'use strict';
+
+// This is a free list to avoid creating so many of the same object.
+exports.FreeList = function(name, max, constructor) {
+  this.name = name;
+  this.constructor = constructor;
+  this.max = max;
+  this.list = [];
+};
+
+
+exports.FreeList.prototype.alloc = function() {
+  //debug("alloc " + this.name + " " + this.list.length);
+  return this.list.length ? this.list.shift() :
+                            this.constructor.apply(this, arguments);
+};
+
+
+exports.FreeList.prototype.free = function(obj) {
+  //debug("free " + this.name + " " + this.list.length);
+  if (this.list.length < this.max) {
+    this.list.push(obj);
+    return true;
+  }
+  return false;
+};
diff --git a/lib/module.js b/lib/module.js
@@ -200,7 +200,7 @@ Module._nodeModulePaths = function(from) {
 
 
 Module._resolveLookupPaths = function(request, parent) {
-  if (NativeModule.exists(request)) {
+  if (NativeModule.nonInternalExists(request)) {
     return [request, []];
   }
 
@@ -262,7 +262,7 @@ Module._load = function(request, parent, isMain) {
     return cachedModule.exports;
   }
 
-  if (NativeModule.exists(filename)) {
+  if (NativeModule.nonInternalExists(filename)) {
     // REPL is a special case, because it needs the real require.
     if (filename == 'repl') {
       var replModule = new Module('repl');
@@ -299,7 +299,7 @@ Module._load = function(request, parent, isMain) {
 };
 
 Module._resolveFilename = function(request, parent) {
-  if (NativeModule.exists(request)) {
+  if (NativeModule.nonInternalExists(request)) {
     return request;
   }
 

diff --git a/node.gyp b/node.gyp
@@ -69,6 +69,8 @@
       'lib/v8.js',
       'lib/vm.js',
       'lib/zlib.js',
+
+      'lib/internal/freelist.js'
     ],
   },
 

diff --git a/src/node.js b/src/node.js
@@ -838,6 +838,14 @@
     return NativeModule._source.hasOwnProperty(id);
   };
 
+  NativeModule.nonInternalExists = function(id) {
+    return NativeModule.exists(id) && !NativeModule.isInternal(id);
+  };
+
+  NativeModule.isInternal = function(id) {
+    return id.startsWith('internal/');
+  };
+
   NativeModule.getSource = function(id) {
     return NativeModule._source[id];
   };

diff --git a/test/fixtures/internal-modules/index.js b/test/fixtures/internal-modules/index.js
@@ -0,0 +1 @@
+module.exports = require('internal/freelist');
diff --git a/test/fixtures/internal-modules/node_modules/internal/freelist.js b/test/fixtures/internal-modules/node_modules/internal/freelist.js
diff --git a/test/parallel/test-internal-modules.js b/test/parallel/test-internal-modules.js
@@ -0,0 +1,8 @@
+var common = require('../common');
+var assert = require('assert');
+
+assert.throws(function() {
+  require('internal/freelist');
+});
+
+assert(require('../fixtures/internal-modules') === true);
diff --git a/tools/js2c.py b/tools/js2c.py
@@ -238,11 +238,11 @@ def ReadMacros(lines):
 
 
 NATIVE_DECLARATION = """\
-  { "%(id)s", %(id)s_native, sizeof(%(id)s_native)-1 },
+  { "%(id)s", %(escaped_id)s_native, sizeof(%(escaped_id)s_native)-1 },
 """
 
 SOURCE_DECLARATION = """\
-  const char %(id)s_native[] = { %(data)s };
+  const char %(escaped_id)s_native[] = { %(data)s };
 """
 
 
@@ -293,16 +293,18 @@ def JS2C(source, target):
     lines = ExpandMacros(lines, macros)
     lines = CompressScript(lines, do_jsmin)
     data = ToCArray(s, lines)
-    id = os.path.basename(str(s)).split('.')[0]
+    id = '/'.join((str(s).split(os.sep)[1:])).split('.')[0]
     if delay: id = id[:-6]
     if delay:
       delay_ids.append((id, len(lines)))
     else:
       ids.append((id, len(lines)))
-    source_lines.append(SOURCE_DECLARATION % { 'id': id, 'data': data })
-    source_lines_empty.append(SOURCE_DECLARATION % { 'id': id, 'data': 0 })
-    native_lines.append(NATIVE_DECLARATION % { 'id': id })
-
+
+    escaped_id = id.replace('/', '$')
+    source_lines.append(SOURCE_DECLARATION % { 'id': id, 'escaped_id': escaped_id, 'data': data })
+    source_lines_empty.append(SOURCE_DECLARATION % { 'id': id, 'escaped_id': escaped_id, 'data': 0 })
+    native_lines.append(NATIVE_DECLARATION % { 'id': id, 'escaped_id': escaped_id })
+
   # Build delay support functions
   get_index_cases = [ ]
   get_script_source_cases = [ ]
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		module.exports = require('internal/freelist');
Copy link Copy Markdown Member bnoordhuis Mar 13, 2015 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. Shouldn't this file be called freelist.js or something? Avoids unnecessary churn when another module is added. Copy link Copy Markdown Contributor Author vkurchatkin Mar 13, 2015 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. Not sure what do you mean. `js2c` drops extensions so this is how we require internal modules. This particular test checks that if we try it in userland module and there is `node_modules/internal/freelist.js` everything works as before. Copy link Copy Markdown Member bnoordhuis Mar 13, 2015 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. The directory is called test/fixtures/internal-modules but it's really only about the freelist module. Maybe move index.js to test/fixtures/internal-modules/freelist? Copy link Copy Markdown Contributor Author vkurchatkin Mar 13, 2015 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. doesn't change a lot. we only need to test this once with any module. it could be anything other than `freelist`