Chai 4/Ember 3.0.0-beta.1 error: You attempted to access the firstObject.inspect property

[Dhaulagiri]

Running tests with Chai4 and Ember 3 beta 1, I'm getting the below error

Error: Assertion Failed: You attempted to access the firstObject.inspect property (of ). Due to certain internal implementation details of Ember, the firstObject property previously contained an internal "descriptor" object (a private API), therefore firstObject.inspect would have been undefined. This internal implementation detail was never intended to be a public (or even intimate) API.

This internal implementation detail has now changed and the (still private) "descriptor" object has been relocated to the object's "meta" (also a private API). Soon, accessing firstObject on this object will return the computed value (see RFC #281 for more details).

The error seems to stem from the way Chai 4's .to.contain logic works in a test like the below:

  let arr;
  beforeEach(function() {
    arr = [
      EmberObject.create({ id: 1 }),
      EmberObject.create({ id: 2 })
    ];
  });

  it('tests', function() {
      expect(arr).to.contain('foo');
  });

The Chai code that blows up is something like this:

if (value && typeof value.inspect === 'function' &&

I'm not sure how to reproduce this via twiddle, but you clone this repo and run the included test you can see the error. Note that if I drop back to Chai 3 the problem goes away.

[rwjblue]

The issue here is that chai@4 is attempting to provide a nice display message for the failing assertion with some custom display logic. To accomplish that they are using a copy/fork of Node's util.inspect() (see here). When invoked, util.inspect() recursively inspects each property, and if an object has an .inspect function it will invoke it essentially as a way to customize the inspection output (ala toString(), but under specific situations).

In Node 7+ the recommended way to customize inspection changed from implementing an .inspect function on your object to using a custom symbol (see here) which our existing descriptor trap already handles properly.

---

I believe the correct solution here is to add inspect to the list of exceptions in our descriptor trap here:

ember.js/packages/ember-metal/lib/properties.js

Line 115 in 23e5386

} else if (property === 'toString' || property == 'valueOf' || (Symbol && property === Symbol.toPrimitive)) {

Mind sending in a PR (should be a [BUGFIX beta])?

[chancancode]

👍

[Dhaulagiri]

Running into a similar issue using chai's deep equal however in this case property is Symbol(Symbol.toStringTag). I can try to add a more detailed reproduction when I have time.

I wish I had a better understanding of what DESCRIPTOR_TRAP is doing here since basically all our test failures in Ember 3 are related to this section of code (and they are not all chai related).

[rwjblue]

@Dhaulagiri - The goal of the assertion is to throw an error when folks "touch" descriptors in a way that shouldn't have been possible, but accidentally was. It is a pretty uncommon situation, but prior to implementing emberjs/rfcs#281 (native ES5 getters for CPs) we wanted to ensure that folks were not relying on "using" descriptors directly from the prototype.

I think an example will make this much simpler:

let Person = Ember.Object.extend({
  name: Ember.computed('first', 'last', function() {
    return `${this.get('first')} ${this.get('last')}`;
  })
});

let person = Person.create({ first: 'Robert', last: 'Jackson' });

// this "worked" (for some definition of worked) in some Ember versions, but should not have
// the descriptor trap is specifically geared towards making this throw a descriptive error
// since they should have used `this.get('name')` or `Ember.get('name')`
person.name.get() 

[Dhaulagiri]

That makes sense. The reason this has been a bit of a head scratcher is that all of the errors I've come across are not actually in our app code directly but are either in Chai as described above or in a jQuery plugin (I need to dig more into this to see what is actually happening).

[rwjblue]

Right, the usage of a Proxy here is somewhat hard to get right actually. While I have had reports of it legitimately catching bugs/errors in test code (where they were accessing a CP like this.propName.somethingElse and it had always previously been undefined but is now throwing an error), I am concerned that the usages of the type you describe (general object probing, and not really related to the specific issues we are trying to flag) are actually going to make this assertion less useful by being extremely noisy.

One possible solution here, would be to remove the branch

ember.js/packages/ember-metal/lib/properties.js

Lines 108 to 123 in 23e5386

	if (HAS_NATIVE_PROXY) {
	/* globals Proxy */
	trapFor = function(obj, keyName, descriptor) {
	return new Proxy(descriptor, {
	get(descriptor, property) {
	if (property === DESCRIPTOR) {
	return descriptor;
	} else if (property === 'toString' || property == 'valueOf' || (Symbol && property === Symbol.toPrimitive)) {
	return () => '[COMPUTED PROPERTY]';
	}
	assert(messageFor(obj, keyName, property, descriptor[property]));
	}
	});
	};
	} else {

And instead just assert when a user accesses one of the known properties that the computed property prototype has, and allow other access to return undefined.

@chancancode - Thoughts?

[rwjblue]

Reopening for now, because even though we landed #16050 which fixes many scenarios, we are still not 100% certain that we have covered all of the cases that @Dhaulagiri was running into.

[Dhaulagiri]

@rwjblue I pushed up another sample test that mimics our app code to the sample repo that reproduces the other error I've seen.

[rwjblue]

Awesome, thank you! I'll try to dig into it this afternoon...

[Dhaulagiri]

This continues to be a game of whack-a-mole. I've added a few exceptions that have been merged in, but as I clean those up I keep hitting new land mines. Another one related to a jquery plugin we are using is:

You attempted to access the [].isDescriptor property (of [object Object],[object Object])

Adding an exception for that then triggers the same error on []._getter

[Dhaulagiri]

We are on an old version of jquery but I believe this same line in the latest version is the same line triggering this for us in our app:

https://github.com/jquery/jquery/blob/1ea092a54b00aa4d902f4e22ada3854d195d4a18/src/core.js#L159

[Dhaulagiri]

I'm going to use this as motivation to remove the jquery plugin in question since I wanted to do that anyway, but the chai issue I reproduced in this comment still lingers

[rwjblue]

@Dhaulagiri - I believe that the recent changes in #16169 should have addressed the remaining issues. I tested your sample repo against the latest beta build which includes those changes, and the tests pass.

[Dhaulagiri]

@rwjblue thank you! I'll be sure to test the next release to confirm but it sounds like we should be able to close this out.

[Dhaulagiri]

Confirmed all chai-related errors are fixed. Cannot confirm jquery one since I removed that plugin from our codebase, but it looks like others will be able to confirm in some of the linked issues.

[rwjblue]

FYI - This fix was just released as part of 3.0.0-beta.4...

[Dhaulagiri]

Confirmed...now to figure out where my new test failures are coming from and try to report them 😭