Avoid instanceof which leads to runtime errors with deep npm dependencies

[jeromesimeon]

The use of instanceof to check whether an object is of a class leads to unreliable check when importing Composer Concerto in projects with deep npm dependencies.

Context

Composer Concerto uses instanceof checks e.g., here https://github.com/hyperledger/composer-concerto/blob/37189e52c4f52d13cbd862b38d36a64022976398/lib/serializer/resourcevalidator.js#L72 or here https://github.com/hyperledger/composer-concerto/blob/37189e52c4f52d13cbd862b38d36a64022976398/lib/serializer/jsonpopulator.js#L96

Those can be unreliable since it checks whether this is the same object from the exact same package installed in a node.js project.

The reason it is unreliable is that it is difficult to enforce that the corresponding version of composer-concerto is installed exactly once in the ./node_modules directory through npm install.

This can lead to runtime errors when doing validation/serialization in some projects with deep npm dependencies.

Expected Behavior

A project should be able to import Composer Concerto 0.7.1 and PROJECT 0.X.X depending on e.g., Composer Concerto 0.7.0 without leading to runtime errors.

Actual Behavior

Currently a project has to ensure every dependency which import Composer Concerto have the exact same version, and that npm install leads to only one copy of that same version, which is difficult to guarantee (due to the way npm install works).

Possible Fix

Several fixes are suggested in: https://stackoverflow.com/questions/41587865/using-instanceof-on-objects-created-with-constructors-from-deep-npm-dependenci

The recommended fix is to use an explicit method to check whether an object has the expected class. So instead of writing thing instanceof RelationshipDeclaration one would write isRelationshipDeclaration(thing) where:

RelationshipDeclaration.prototype._isRelationshipDeclaration = true;
var isRelationshipDeclaration = function isRelationshipDeclaration(obj) {
  return obj instanceof RelationshipDeclaration || Boolean(obj._isRelationshipDeclaration);
};

module.exports = {
  RelationshipDeclaration,
  isRelationshipDeclaration
};

Steps to Reproduce

Existing issues

• Stack Overflow issues
• GitHub Issues
• Rocket Chat history

Context

Importing Composer Concerto in various parts of the Accord Project (Ergo, Cicero, Cicero template library, template studio, etc.).

Your Environment

• Version used: 0.70.*
• Environment name and version (e.g. Chrome 39, node.js 5.4): node.js 8
• Operating System and version (desktop or mobile): MacOS
• Link to your project:

[jeromesimeon]

There still seems to be a few occurrences of instance of around:

bash-3.2$ grep -r instanceof lib
lib/serializer/typedstack.js:        if(expectedType && !(obj instanceof expectedType)) {
lib/serializer/typedstack.js:        if(expectedType && !(result instanceof expectedType)) {
lib/serializer/jsonpopulator.js:        if (thing instanceof ClassDeclaration) {
lib/serializer/jsonpopulator.js:        } else if (thing instanceof RelationshipDeclaration) {
lib/serializer/jsonpopulator.js:        } else if (thing instanceof Field) {
lib/serializer/instancegenerator.js:        if (thing instanceof ClassDeclaration) {
lib/serializer/instancegenerator.js:        } else if (thing instanceof RelationshipDeclaration) {
lib/serializer/instancegenerator.js:        } else if (thing instanceof Field) {
lib/serializer/instancegenerator.js:        if (classDeclaration instanceof EnumDeclaration) {
lib/serializer/resourcevalidator.js:        if (thing instanceof EnumDeclaration) {
lib/serializer/resourcevalidator.js:        } else if (thing instanceof ClassDeclaration) {
lib/serializer/resourcevalidator.js:        } else if (thing instanceof RelationshipDeclaration) {
lib/serializer/resourcevalidator.js:        } else if (thing instanceof Field) {
lib/serializer/resourcevalidator.js:        if(!((obj instanceof Resource) || (obj instanceof Concept))) {
lib/serializer/resourcevalidator.js:        if(obj instanceof Identifiable) {
lib/serializer/resourcevalidator.js:                    if(obj instanceof Identifiable) {
lib/serializer/resourcevalidator.js:        if(obj instanceof Identifiable) {
lib/serializer/resourcevalidator.js:        if(field.isArray() && !(obj instanceof Array)) {
lib/serializer/resourcevalidator.js:        if(!(obj instanceof Array)) {
lib/serializer/resourcevalidator.js:            if(obj instanceof Identifiable) {
lib/serializer/resourcevalidator.js:            if(!(obj instanceof Array)) {
lib/serializer/resourcevalidator.js:        if(obj instanceof Relationship) {
lib/serializer/resourcevalidator.js:        } else if (obj instanceof Resource && (this.options.convertResourcesToRelationships || this.options.permitResourcesForRelationships)) {
lib/serializer/resourcevalidator.js:        if(value instanceof Identifiable) {
lib/serializer/jsongenerator.js:        if (thing instanceof ClassDeclaration) {
lib/serializer/jsongenerator.js:        } else if (thing instanceof RelationshipDeclaration) {
lib/serializer/jsongenerator.js:        } else if (thing instanceof Field) {
lib/serializer/jsongenerator.js:        if (!((obj instanceof Resource) || (obj instanceof Concept))) {
lib/serializer/jsongenerator.js:        if (obj instanceof Identifiable && this.deduplicateResources) {
lib/serializer/jsongenerator.js:                if (this.permitResourcesForRelationships && item instanceof Resource) {
lib/serializer/jsongenerator.js:        } else if (this.permitResourcesForRelationships && obj instanceof Resource) {
lib/serializer/jsongenerator.js:        if (relationshipOrResource instanceof Resource) {
lib/factory.js:        if (!(classDeclaration instanceof TransactionDeclaration)) {
lib/factory.js:        if (!(classDeclaration instanceof EventDeclaration)) {
lib/introspect/parser.pegjs:InstanceofToken = "instanceof" !IdentifierPart
lib/introspect/modelfile.js:        if(classDeclaration instanceof AssetDeclaration) {
lib/introspect/modelfile.js:        if(classDeclaration instanceof TransactionDeclaration) {
lib/introspect/modelfile.js:        if(classDeclaration instanceof EventDeclaration) {
lib/introspect/modelfile.js:        if(classDeclaration instanceof ParticipantDeclaration) {
lib/introspect/modelfile.js:            if(classDeclaration instanceof type && (includeSystemType || !classDeclaration.isSystemType())) {
lib/introspect/parser.js:            escapedParts += expectation.parts[i] instanceof Array
lib/introspect/parser.js:      peg$c182 = "instanceof",
lib/introspect/parser.js:      peg$c183 = peg$literalExpectation("instanceof", false),
lib/model/typed.js:            if (field instanceof Field) {
lib/serializer.js:        if(!(resource instanceof Typed)) {
lib/serializer.js:        if (classDeclaration instanceof TransactionDeclaration) {
lib/serializer.js:        } else if (classDeclaration instanceof EventDeclaration) {
lib/serializer.js:        } else if (classDeclaration instanceof ConceptDeclaration) {
lib/serializer.js:        } else if (classDeclaration instanceof EnumDeclaration) {

[jeromesimeon]

Confirming that this works. The approach is to override hasInstance in the corresponding classes, which effectively underlies the implementation for instanceof.

[dselman]

Reopening based on further investigation and how it interacts with Babel:
accordproject/web-components#110