this would mean chai extensions (properties, methods, etc) get overwritten/added each time right?

do we know what our suggestion would be to plugin authors for ensuring they don't accidentally duplicate things?

just thinking of a situation where they add a method which wraps the original. so if you call it many times, you end up with customMethod(customMethod(originalMethod(...args))) basically (which would probably work in many cases but be not great)

this would mean chai extensions (properties, methods, etc) get overwritten/added each time right?

I think so. Isn't that how it worked in chai@4 as well?

do we know what our suggestion would be to plugin authors for ensuring they don't accidentally duplicate things?

just thinking of a situation where they add a method which wraps the original. so if you call it many times, you end up with customMethod(customMethod(originalMethod(...args))) basically (which would probably work in many cases but be not great)

I'm probably missing something so maybe we can express this as a test? Something like:

function somePlugin(chai) {
  if (chai.Assertion.prototype.testing) return;

  Object.defineProperty(chai.Assertion.prototype, 'testing', {
    get: function () {
      return 'successful';
    },
  });
}

function someOtherPlugin(chai) {
  if (chai.Assertion.prototype.testing) return;

  Object.defineProperty(chai.Assertion.prototype, 'testing', {
    get: function () {
      return 'failed';
    },
  });
}

let chai = use(somePlugin);
chai = chai.use(someOtherPlugin);

expect(chai.myProperty).to.equal('successful');

in chai 4 it would've only called it once afaik (thats what used was for)

i may be talking nonsense but maybe someone does something like this:

chai.overwriteProperty('equal', function (_super) {
  if (something) {
    return _super.call(this);
  }
  return someOtherThing();
});

which presumably, when called many times in some cases, would result in _super(_super(_super())) etc

so it may be worth us documenting some recommended way of avoiding that, i.e. checking you already overwrote the property, added the method, etc

You're probably right and it's something I haven't though of. Without a bit more concrete test I'm just not following 😅

I added the example test from your comment in 44af64d but I feel like I'm missing something obvious.

basically im not sure if overwriteProperty multiple times results in the original being super or the last overwritten one

given the example before:

chai.overwriteProperty('equal', function (_super) {
  if (something) {
    return _super.call(this);
  }
  return someOtherThing();
});

i wonder if _super is the current property, not the original

so if you did this:

const handler = function (_super) {
  console.log('Called!'); // log something out

  return _super.call(this);
};

chai.overwriteProperty('equal', handler);
chai.overwriteProperty('equal', handler); // equivalent to you calling `use` twice

chai.expect(123).to.equal(123);

does it log Called! twice or once?

im just saying, if it calls it twice, we should probably document a recommended way of avoiding registering the same thing twice

does it make sense now?

does it make sense now?

I think I'm getting it. I was hoping to run some of the code so I could poke and prod and compare chai@4 and chai@5 but I can't get anything to work. Running this in chai@4 results in a error:

const chai = require("chai");

function somePlugin(chai) {
  chai.util.overwriteProperty(chai.Assertion.prototype, "equal", function (_super) {
    console.log("Called!"); // log something out

    console.log(_super);
    return _super.call(this);
  });
}

chai.use(somePlugin);
chai.use(somePlugin);

chai.expect(123).to.equal(123);

im just saying, if it calls it twice, we should probably document a recommended way of avoiding registering the same thing twice

So document things like https://github.com/chaijs/chai/pull/1617/files#diff-919c27039f4865fa252acfbd90d91c8b6c33b42dbf3d593fac7648cc6aa7a3f5R4 but maybe a bit more reliable? Otherwise we could also document that people should be vary of registering the same plugin twice as that can result in undefined behaviour?

i just tried it and it seems to call it once in both 5 and 4

its because its a method (equal), not a property:

import * as chai from 'chai';

function somePlugin(base) {
  base.util.overwriteMethod(base.Assertion.prototype, "equal", function (_super) {
    return function(...args) {
      console.log("Called!"); // log something out

      return _super.call(this, ...args);
    };
  });
}

chai.use(somePlugin);
chai.use(somePlugin);

chai.expect(123).to.equal(123);

so i think you can ignore me and all is fine

edit: nevermind, with your change it does call it twice. so we should probably document that as the plugin author's responsibility

basically this is what i'd tell authors to do (overwritten flag):

import * as chai from 'chai';

let overwritten = false;

function somePlugin(base) {
  if (!overwritten) {
    base.util.overwriteMethod(base.Assertion.prototype, "equal", function (_super) {
      return function(...args) {
        console.log("Called!"); // log something out

        return _super.call(this, ...args);
      };
    });
    overwritten = true;
  }
}

chai.use(somePlugin);
chai.use(somePlugin);

chai.expect(123).to.equal(123); // Logs `Called!` only once

I'm cleaning up some old PRs and am gonna close this. I'll re-open a different one if I end up working on this.