Skip to content

fix: ensure that types are always resolved #2068

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
May 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 3 additions & 15 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,9 @@ export abstract class NamespaceBase extends ReflectionObject {
/** Whether or not objects contained in this namespace need feature resolution. */
protected _needsRecursiveFeatureResolution: boolean;

/** Whether or not objects contained in this namespace need a resolve. */
protected _needsRecursiveResolve: boolean;

/** Nested objects of this namespace as an array for iteration. */
public readonly nestedArray: ReflectionObject[];

Expand Down Expand Up @@ -900,21 +903,6 @@ export abstract class ReflectionObject {
/** Unique name within its namespace. */
public name: string;

/** The edition specified for this object. Only relevant for top-level objects. */
public _edition: string;

/**
* The default edition to use for this object if none is specified. For legacy reasons,
* this is proto2 except in the JSON parsing case where it was proto3.
*/
public _defaultEdition: string;

/** Resolved Features. */
public _features: object;

/** Whether or not features have been resolved. */
public _featuresResolved: boolean;

/** Parent namespace. */
public parent: (Namespace|null);

Expand Down
50 changes: 41 additions & 9 deletions src/namespace.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,13 @@ function Namespace(name, options) {
* @protected
*/
this._needsRecursiveFeatureResolution = true;

/**
* Whether or not objects contained in this namespace need a resolve.
* @type {boolean}
* @protected
*/
this._needsRecursiveResolve = true;
}

function clearCache(namespace) {
Expand Down Expand Up @@ -273,11 +280,13 @@ Namespace.prototype.add = function add(object) {
}

this._needsRecursiveFeatureResolution = true;
this._needsRecursiveResolve = true;

// Also clear parent caches, since they need to recurse down.
var parent = this;
while(parent = parent.parent) {
parent._needsRecursiveFeatureResolution = true;
parent._needsRecursiveResolve = true;
}

object.onAdd(this);
Expand Down Expand Up @@ -341,13 +350,16 @@ Namespace.prototype.define = function define(path, json) {
* @returns {Namespace} `this`
*/
Namespace.prototype.resolveAll = function resolveAll() {
if (!this._needsRecursiveResolve) return this;

var nested = this.nestedArray, i = 0;
this.resolve();
while (i < nested.length)
if (nested[i] instanceof Namespace)
nested[i++].resolveAll();
else
nested[i++].resolve();
this._needsRecursiveResolve = false;
return this;
};

Expand Down Expand Up @@ -389,29 +401,47 @@ Namespace.prototype.lookup = function lookup(path, filterTypes, parentAlreadyChe
} else if (!path.length)
return this;

var flatPath = path.join(".");

// Start at root if path is absolute
if (path[0] === "")
return this.root.lookup(path.slice(1), filterTypes);

var found = this._lookupImpl(path);
// Early bailout for objects with matching absolute paths
var found = this.root._fullyQualifiedObjects["." + flatPath];
if (found && (!filterTypes || filterTypes.indexOf(found.constructor) > -1)) {
return found;
}

// Do a regular lookup at this namespace and below
found = this._lookupImpl(path, flatPath);
if (found && (!filterTypes || filterTypes.indexOf(found.constructor) > -1)) {
return found;
}

// If there hasn't been a match, try again at the parent
if (this.parent === null || parentAlreadyChecked)
if (parentAlreadyChecked)
return null;
return this.parent.lookup(path, filterTypes);

// If there hasn't been a match, walk up the tree and look more broadly
var current = this;
while (current.parent) {
found = current.parent._lookupImpl(path, flatPath);
if (found && (!filterTypes || filterTypes.indexOf(found.constructor) > -1)) {
return found;
}
current = current.parent;
}
return null;
};

/**
* Internal helper for lookup that handles searching just at this namespace and below along with caching.
* @param {string[]} path Path to look up
* @param {string} flatPath Flattened version of the path to use as a cache key
* @returns {ReflectionObject|null} Looked up object or `null` if none could be found
* @private
*/
Namespace.prototype._lookupImpl = function lookup(path) {
var flatPath = path.join(".");
Namespace.prototype._lookupImpl = function lookup(path, flatPath) {
if(Object.prototype.hasOwnProperty.call(this._lookupCache, flatPath)) {
return this._lookupCache[flatPath];
}
Expand All @@ -422,13 +452,15 @@ Namespace.prototype._lookupImpl = function lookup(path) {
if (found) {
if (path.length === 1) {
exact = found;
} else if (found instanceof Namespace && (found = found._lookupImpl(path.slice(1))))
exact = found;
} else if (found instanceof Namespace) {
path = path.slice(1);
exact = found._lookupImpl(path, path.join("."));
}

// Otherwise try each nested namespace
} else {
for (var i = 0; i < this.nestedArray.length; ++i)
if (this._nestedArray[i] instanceof Namespace && (found = this._nestedArray[i]._lookupImpl(path)))
if (this._nestedArray[i] instanceof Namespace && (found = this._nestedArray[i]._lookupImpl(path, flatPath)))
exact = found;
}

Expand Down
4 changes: 4 additions & 0 deletions src/object.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,25 +51,29 @@ function ReflectionObject(name, options) {
/**
* The edition specified for this object. Only relevant for top-level objects.
* @type {string}
* @private
*/
this._edition = null;

/**
* The default edition to use for this object if none is specified. For legacy reasons,
* this is proto2 except in the JSON parsing case where it was proto3.
* @type {string}
* @private
*/
this._defaultEdition = "proto2";

/**
* Resolved Features.
* @type {object}
* @private
*/
this._features = {};

/**
* Whether or not features have been resolved.
* @type {boolean}
* @private
*/
this._featuresResolved = false;

Expand Down
28 changes: 24 additions & 4 deletions src/root.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,19 @@ function Root(options) {
*/
this.files = [];

// Default to proto2 if unspecified.
/**
* Edition, defaults to proto2 if unspecified.
* @type {string}
* @private
*/
this._edition = "proto2";

/**
* Global lookup cache of fully qualified names.
* @type {Object.<string,ReflectionObject>}
* @private
*/
this._fullyQualifiedObjects = {};
}

/**
Expand All @@ -51,7 +62,7 @@ Root.fromJSON = function fromJSON(json, root) {
root = new Root();
if (json.options)
root.setOptions(json.options);
return root.addJSON(json.nested)._resolveFeaturesRecursive();
return root.addJSON(json.nested).resolveAll();
};

/**
Expand Down Expand Up @@ -100,7 +111,7 @@ Root.prototype.load = function load(filename, options, callback) {
// Finishes loading by calling the callback (exactly once)
function finish(err, root) {
if (root) {
root._resolveFeaturesRecursive();
root.resolveAll();
}
/* istanbul ignore if */
if (!callback) {
Expand Down Expand Up @@ -219,7 +230,7 @@ Root.prototype.load = function load(filename, options, callback) {
if (resolved = self.resolvePath("", filename[i]))
fetch(resolved);
if (sync) {
self._resolveFeaturesRecursive();
self.resolveAll();
return self;
}
if (!queued) {
Expand Down Expand Up @@ -268,6 +279,8 @@ Root.prototype.loadSync = function loadSync(filename, options) {
* @override
*/
Root.prototype.resolveAll = function resolveAll() {
if (!this._needsRecursiveResolve) return this;

if (this.deferred.length)
throw Error("unresolvable extensions: " + this.deferred.map(function(field) {
return "'extend " + field.extend + "' in " + field.parent.fullName;
Expand Down Expand Up @@ -335,6 +348,11 @@ Root.prototype._handleAdd = function _handleAdd(object) {
object.parent[object.name] = object; // expose namespace as property of its parent
}

if (object instanceof Type || object instanceof Enum || object instanceof Field) {
// Only store types and enums for quick lookup during resolve.
this._fullyQualifiedObjects[object.fullName] = object;
}

// The above also adds uppercased (and thus conflict-free) nested types, services and enums as
// properties of namespaces just like static code does. This allows using a .d.ts generated for
// a static module with reflection-based solutions where the condition is met.
Expand Down Expand Up @@ -375,6 +393,8 @@ Root.prototype._handleRemove = function _handleRemove(object) {
delete object.parent[object.name]; // unexpose namespaces

}

delete this._fullyQualifiedObjects[object.fullName];
};

// Sets up cyclic dependencies (called in index-light)
Expand Down
2 changes: 2 additions & 0 deletions src/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ Service.prototype.get = function get(name) {
* @override
*/
Service.prototype.resolveAll = function resolveAll() {
if (!this._needsRecursiveResolve) return this;

Namespace.prototype.resolve.call(this);
var methods = this.methodsArray;
for (var i = 0; i < methods.length; ++i)
Expand Down
2 changes: 2 additions & 0 deletions src/type.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,8 @@ Type.prototype.toJSON = function toJSON(toJSONOptions) {
* @override
*/
Type.prototype.resolveAll = function resolveAll() {
if (!this._needsRecursiveResolve) return this;

Namespace.prototype.resolveAll.call(this);
var oneofs = this.oneofsArray; i = 0;
while (i < oneofs.length)
Expand Down