Refactor import hook API to subsume all platform-dependent behavior

etc/browser/lib/files.js

@@ -5,7 +5,7 @@
 function createError() { return new Error('unsupported in the browser'); }
 
 function createImportHook() {
-  return function (fpath, kind, cb) { cb(createError()); };
+  return function (_, cb) { cb(createError()); };
 }
 
 function createSyncImportHook() {

lib/files.js

@@ -13,15 +13,18 @@ let fs = require('fs'),
 /** Default (asynchronous) file loading function for assembling IDLs. */
 function createImportHook() {
   let imports = {};
-  return function (fpath, kind, cb) {
-    fpath = path.resolve(fpath);
+  return function ({path: fpath, parentPath}, cb) {
+    fpath = path.resolve(path.dirname(parentPath), fpath);
     if (imports[fpath]) {
       // Already imported, return nothing to avoid duplicating attributes.
       process.nextTick(cb);
       return;
     }
     imports[fpath] = true;
-    fs.readFile(fpath, {encoding: 'utf8'}, cb);
+    fs.readFile(fpath, {encoding: 'utf8'}, (err, data) => {
+      if (err) return cb(err);
+      return cb(null, {contents: data, path: fpath});
+    });
   };
 }
 
@@ -35,13 +38,16 @@ function createImportHook() {
  */
 function createSyncImportHook() {
   let imports = {};
-  return function (fpath, kind, cb) {
-    fpath = path.resolve(fpath);
+  return function ({path: fpath, parentPath}, cb) {
+    fpath = path.resolve(path.dirname(parentPath), fpath);
     if (imports[fpath]) {
       cb();
     } else {
       imports[fpath] = true;
-      cb(null, fs.readFileSync(fpath, {encoding: 'utf8'}));
+      cb(null, {
+        contents: fs.readFileSync(fpath, {encoding: 'utf8'}),
+        path: fpath
+      });
     }
   };
 }

lib/specs.js

@@ -7,8 +7,7 @@
 /** IDL to protocol (services) and schema (types) parsing logic. */
 
 let files = require('./files'),
-    utils = require('./utils'),
-    path = require('path');
+    utils = require('./utils');
 
 
 // Default type references defined by Avro.
@@ -31,7 +30,7 @@ function assembleProtocol(fpath, opts, cb) {
     opts.importHook = files.createImportHook();
   }
 
-  importFile(fpath, (err, protocol) => {
+  importFile(fpath, '', (err, protocol) => {
     if (err) {
       cb(err);
       return;
@@ -57,19 +56,20 @@ function assembleProtocol(fpath, opts, cb) {
     cb(null, protocol);
   });
 
-  function importFile(fpath, cb) {
-    opts.importHook(fpath, 'idl', (err, str) => {
+  function importFile(fpath, parentPath, cb) {
+    opts.importHook({path: fpath, parentPath, kind: 'idl'}, (err, params) => {
       if (err) {
         cb(err);
         return;
       }
-      if (str === undefined) {
+      if (!params) {
         // This signals an already imported file by the default import hooks.
         // Implementors who wish to disallow duplicate imports should provide a
         // custom hook which throws an error when a duplicate is detected.
         cb();
         return;
       }
+      const {contents: str, path: fpath} = params;
       let obj;
       try {
         let reader = new Reader(str, opts);
@@ -79,11 +79,11 @@ function assembleProtocol(fpath, opts, cb) {
         cb(err);
         return;
       }
-      fetchImports(obj.protocol, obj.imports, path.dirname(fpath), cb);
+      fetchImports(obj.protocol, obj.imports, fpath, cb);
     });
   }
 
-  function fetchImports(protocol, imports, dpath, cb) {
+  function fetchImports(protocol, imports, fpath, cb) {
     let importedProtocols = [];
     next();
 
@@ -104,9 +104,8 @@ function assembleProtocol(fpath, opts, cb) {
         cb(null, protocol);
         return;
       }
-      let importPath = path.join(dpath, info.name);
       if (info.kind === 'idl') {
-        importFile(importPath, (err, imported) => {
+        importFile(info.name, fpath, (err, imported) => {
           if (err) {
             cb(err);
             return;
@@ -118,24 +117,28 @@ function assembleProtocol(fpath, opts, cb) {
         });
       } else {
         // We are importing a protocol or schema file.
-        opts.importHook(importPath, info.kind, (err, str) => {
+        opts.importHook({
+          path: info.name,
+          parentPath: fpath,
+          kind: info.kind
+        }, (err, params) => {
           if (err) {
             cb(err);
             return;
           }
           switch (info.kind) {
             case 'protocol':
             case 'schema': {
-              if (str === undefined) {
+              if (!params) {
                 // Skip duplicate import (see related comment above).
                 next();
                 return;
               }
               let obj;
               try {
-                obj = JSON.parse(str);
+                obj = JSON.parse(params.contents);
               } catch (err) {
-                err.path = importPath;
+                err.path = params.path;
                 cb(err);
                 return;
               }
@@ -193,7 +196,7 @@ function assembleProtocol(fpath, opts, cb) {
  *
  * The parsing logic is as follows:
  *
- * + If `str` contains `path.sep` (on windows `\`, otherwise `/`) and is a path
+ * + If `str` contains `/` and is a path
  *   to an existing file, it will first be read as JSON, then as an IDL
  *   specification if JSON parsing failed. If either succeeds, the result is
  *   returned, otherwise the next steps are run using the file's content
@@ -209,7 +212,7 @@ function read(str) {
   let schema;
   if (
     typeof str == 'string' &&
-    ~str.indexOf(path.sep) &&
+    str.indexOf('/') !== -1 &&
     files.existsSync(str)
   ) {
     // Try interpreting `str` as path to a file contain a JSON schema or an IDL

test/test_specs.js

@@ -449,9 +449,12 @@ suite('specs', () => {
     });
 
     test('import hook error', (done) => {
-      let hook = function (fpath, kind, cb) {
+      let hook = function ({path: fpath}, cb) {
         if (path.basename(fpath) === 'A.avdl') {
-          cb(null, 'import schema "hi"; protocol A {}');
+          cb(null, {
+            contents: 'import schema "hi"; protocol A {}',
+            path: fpath
+          });
         } else {
           cb(new Error('foo'));
         }
@@ -463,9 +466,12 @@ suite('specs', () => {
     });
 
     test('import hook idl error', (done) => {
-      let hook = function (fpath, kind, cb) {
+      let hook = function ({path: fpath}, cb) {
         if (path.basename(fpath) === 'A.avdl') {
-          cb(null, 'import idl "hi"; protocol A {}');
+          cb(null, {
+            contents: 'import idl "hi"; protocol A {}',
+            path: fpath
+          });
         } else {
           cb(new Error('bar'));
         }
@@ -624,11 +630,14 @@ suite('specs', () => {
 
     // Import hook from strings.
     function createImportHook(imports) {
-      return function (fpath, kind, cb) {
-        let key = path.normalize(fpath);
+      return function ({path: fpath, parentPath}, cb) {
+        let key = path.normalize(path.join(path.dirname(parentPath), fpath));
         let str = imports[key];
         delete imports[key];
-        process.nextTick(() => { cb(null, str); });
+        process.nextTick(() => { cb(null, typeof str === 'string' ? {
+          contents: str,
+          path: key
+        } : undefined); });
       };
     }
 

types/index.d.ts

@@ -127,8 +127,18 @@ interface IsValidOptions {
   noUndeclaredFields: boolean;
   errorHook: (path: string[], val: any, type: Type) => void
 }
+
+interface ImportHookParams {
+  path: string;
+  type: 'idl' | 'protocol' | 'schema';
+}
+
+type ImportHookCallback = (err: any, params?: {contents: string, path: string}) => void;
+
+type ImportHook = (params: ImportHookParams, cb: ImportHookCallback) => void;
+
 interface AssembleOptions {
-  importHook: (filePath: string, type: 'idl', callback: Callback<object>) => void;
+  importHook: (params: ImportHookParams, callback: Callback<object>) => void;
 }
 
 interface SchemaOptions {