Skip to content
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

Support for chai v5 #54

Open
attekemppila opened this issue Jan 15, 2024 · 5 comments
Open

Support for chai v5 #54

attekemppila opened this issue Jan 15, 2024 · 5 comments

Comments

@attekemppila
Copy link

attekemppila commented Jan 15, 2024

node v18.19.0
npm v10.2.3

Current peerDependency doesn't include chai v5:

dirty-chai/package.json

Lines 39 to 41 in bb5d007

"peerDependencies": {
"chai": ">=2.2.1 <5"
},

npm install fails because of that.

Does anything else need chaning in the code. I didn't test yet myself.

@perrin4869
Copy link
Contributor

I started looking into this, the main problem right now is that we cannot monkey-patch the utils object anymore, since it is a non-configurable getter, so in chai v5 this will throw.
The solution is to remove the 3 money patches and expect users to install dirty-chai as the last plugin, or to make the getter configurable in chai@5.
@keithamus any opinions?
@joshperry do you still have write permissions to this repo?

@keithamus
Copy link

Happy to make getters configurable in chai. Please file an issue with the explanation on the chai issue tracker so we can track the work there (or better yet, please file a PR 😉).

@lehni
Copy link

lehni commented Aug 1, 2024

I just looked into it. These getters are not produced by chai's own code, but by the esbuild command. I don't think there's an easy way to make them configurable through esbuild:

var __export = (target, all) => {
  for (var name in all)
    __defProp(target, name, { get: all[name], enumerable: true });
};

// (disabled):util
var require_util = __commonJS({
  "(disabled):util"() {
  }
});

// lib/chai/utils/index.js
var utils_exports = {};
__export(utils_exports, {
  addChainableMethod: () => addChainableMethod,
  addLengthGuard: () => addLengthGuard,
  addMethod: () => addMethod,
  addProperty: () => addProperty,
  checkError: () => check_error_exports,
  compareByInspect: () => compareByInspect,
  eql: () => deep_eql_default,
  expectTypes: () => expectTypes,
  flag: () => flag,
  getActual: () => getActual,
  getMessage: () => getMessage2,
  getName: () => getName,
  getOperator: () => getOperator,
  getOwnEnumerableProperties: () => getOwnEnumerableProperties,
  getOwnEnumerablePropertySymbols: () => getOwnEnumerablePropertySymbols,
  getPathInfo: () => getPathInfo,
  hasProperty: () => hasProperty,
  inspect: () => inspect2,
  isNaN: () => isNaN2,
  isProxyEnabled: () => isProxyEnabled,
  isRegExp: () => isRegExp2,
  objDisplay: () => objDisplay,
  overwriteChainableMethod: () => overwriteChainableMethod,
  overwriteMethod: () => overwriteMethod,
  overwriteProperty: () => overwriteProperty,
  proxify: () => proxify,
  test: () => test,
  transferFlags: () => transferFlags,
  type: () => type
});

@keithamus
Copy link

We could also look into providing hooks into this to allow plugins like this one to do what it does, perhaps an onChainableMethodAdded() function or something?

@lehni
Copy link

lehni commented Aug 1, 2024

Yes I was thinking about this too. It would need to cover these hooks:

// Hook new property creations
var addProperty = util.addProperty;
util.addProperty = function(ctx, name, getter) {
addProperty.apply(util, arguments);
// Convert to chained property
convertAssertionPropertyToChainMethod(name, getter);
};
// Hook new method assertions
var addMethod = util.addMethod;
util.addMethod = function(ctx, name) {
addMethod.apply(util, arguments);
Assertion.overwriteMethod(name, function(_super) {
return function() {
var result = execDeferred(this);
return _super.apply(result, arguments);
};
});
};
// Hook new chainable methods
var addChainableMethod = util.addChainableMethod;
util.addChainableMethod = function(ctx, name) {
// When overwriting an existing property, don't patch it
var patch = true;
if(Assertion.prototype.hasOwnProperty(name)) {
patch = false;
}
addChainableMethod.apply(util, arguments);
if(patch) {
Assertion.overwriteChainableMethod(name, function(_super) {
return function() {
// Method call of the chainable method
var result = execDeferred(this);
return _super.apply(result, arguments);
};
}, function(_super) {
return function() {
// Getter of chainable method
defer(this, _super);
return this;
};
});
}
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants