Skip to content

Potential for unexpected behavior in utils.inherits regarding pre-existing child prototype properties and constructor #951

@xzyingxiashubro

Description

@xzyingxiashubro

ve been looking into the utils.inherits helper function and how it sets up prototype chains. I encountered some behavior that might be worth discussing, particularly concerning how it interacts with properties already on the child's prototype and the constructor property itself.
The current implementation of inherits in lib/utils.js appears to be:

exports.inherits = function (ctor, superCtor) {
    var Obj = function() {};
    Obj.prototype = superCtor.prototype;
    ctor.prototype = new Obj();
};

his approach effectively establishes the prototype link. However, the line ctor.prototype = new Obj(); completely replaces the ctor.prototype object.
This has a couple of potential implications:

  1. Loss of Pre-existing Child Prototype Properties: If methods or properties are added to ctor.prototype before inherits(ctor, superCtor) is called, they seem to be lost because the original prototype object is discarded. For instance, in a scenario like this:
function Parent() {}
function Child() {}
Child.prototype.myMethod = function() { /* ... */ }; // Defined before inherits
utils.inherits(Child, Parent);
// const c = new Child();
// c.myMethod(); // This would likely be undefined

The test case it('should work with constructors that have prototype methods') in inherits.test.cjs seems to highlight this, as child.childMethod is unexpectedly undefined.

  1. Constructor Property on Child's Prototype: After ctor.prototype is replaced, the new ctor.prototype.constructor property would naturally point to Obj (the temporary constructor used internally). While standard inheritance patterns often involve resetting ctor.prototype.constructor = ctor; after establishing the prototype chain, the current utils.inherits doesn't seem to include this step. The test cases it('should properly set up prototype inheritance between constructors') and it('should properly maintain the constructor property') in inherits.test.cjs assert that Child.prototype.constructor is Child. Given the inherits implementation, these assertions might not hold true, as Child.prototype.constructor would likely be Obj.

It's possible this behavior is by design for specific internal use cases, or that the convention within JSZip is to always add methods to the child's prototype after calling utils.inherits.
However, if the inherits function is intended for more general use or aims to align with common JavaScript inheritance patterns (like those facilitated by Node.js's util.inherits in the past, or Object.create), these aspects might lead to unexpected outcomes for developers or within the library itself if not carefully managed.
Perhaps this is an area where the function's behavior could be clarified in comments, or if broader compatibility is desired, its implementation could be revisited to preserve existing child prototype members and explicitly set the constructor property.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions