Skip to content

Commit

Permalink
Update import hook to provide path to callback
Browse files Browse the repository at this point in the history
This lets us move all path resolution into the import hook, making it
independent of Node's `fs` module.
  • Loading branch information
valadaptive committed Jan 14, 2024
1 parent f7624a7 commit 03391e5
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 24 deletions.
2 changes: 1 addition & 1 deletion doc
Submodule doc updated from 99654b to 637213
18 changes: 12 additions & 6 deletions lib/files.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,18 @@ let fs = require('fs'),
/** Default (asynchronous) file loading function for assembling IDLs. */
function createImportHook() {
let imports = {};
return function ({path: fpath}, 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});
});
};
}

Expand All @@ -35,13 +38,16 @@ function createImportHook() {
*/
function createSyncImportHook() {
let imports = {};
return function ({path: fpath}, 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
});
}
};
}
Expand Down
28 changes: 16 additions & 12 deletions lib/specs.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function assembleProtocol(fpath, opts, cb) {
opts.importHook = files.createImportHook();
}

importFile(fpath, (err, protocol) => {
importFile(fpath, '', (err, protocol) => {
if (err) {
cb(err);
return;
Expand All @@ -57,19 +57,20 @@ function assembleProtocol(fpath, opts, cb) {
cb(null, protocol);
});

function importFile(fpath, cb) {
opts.importHook({path: fpath, kind: '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);
Expand All @@ -79,11 +80,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();

Expand All @@ -104,9 +105,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;
Expand All @@ -118,24 +118,28 @@ function assembleProtocol(fpath, opts, cb) {
});
} else {
// We are importing a protocol or schema file.
opts.importHook({path: importPath, kind: 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;
}
Expand Down
19 changes: 14 additions & 5 deletions test/test_specs.js
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,10 @@ suite('specs', () => {
test('import hook error', (done) => {
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'));
}
Expand All @@ -465,7 +468,10 @@ suite('specs', () => {
test('import hook idl error', (done) => {
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'));
}
Expand Down Expand Up @@ -624,11 +630,14 @@ suite('specs', () => {

// Import hook from strings.
function createImportHook(imports) {
return function ({path: fpath}, 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); });
};
}

Expand Down
4 changes: 4 additions & 0 deletions types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ interface ImportHookParams {
type: 'idl' | 'protocol' | 'schema';
}

type ImportHookCallback = (err: any, params?: {contents: string, path: string}) => void;

type ImportHook = (params: ImportHookParams, cb: ImportHookCallback) => void;

interface AssembleOptions {
importHook: (params: ImportHookParams, callback: Callback<object>) => void;
}
Expand Down

0 comments on commit 03391e5

Please sign in to comment.