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.

Sign up for GitHub

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

Jump to bottom

[BUGFIX release] Prevent errors in ember-engines + 3.1 + proxies. #16613

Merged
merged 1 commit into from
May 8, 2018
Merged

[BUGFIX release] Prevent errors in ember-engines + 3.1 + proxies. #16613

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/ember-runtime/lib/system/core_object.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ function makeCtor(base) {
property === 'didAddListener' ||
property === 'didRemoveListener' ||
property === 'isDescriptor' ||
property === '_onLookup' ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we were just going to exclude type 'function' in addition to undefined

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We did that, but as you can see in this case the unknownProperty always returns non-null non-undefined.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't it return a function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not on the instance which is what this test is doing. The test replicates how ember-engines passes down services from the host app into the engines own container. Roughly akin to:

instance = hostAppOwner.lookup(name);
engineOwner.register(name, instance, { instantiate: false })

container.lookup uses factoryFor first (so it can later call .create()) and that does the _onLookup probe on the result (an instance in this case).

property in target
) {
return Reflect.get(target, property, receiver);
Expand Down
20 changes: 19 additions & 1 deletion packages/ember-runtime/tests/system/core_object_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { getOwner, setOwner } from 'ember-owner';
import { get } from 'ember-metal';
import CoreObject from '../../lib/system/core_object';
import { moduleFor, AbstractTestCase } from 'internal-test-helpers';
import { moduleFor, AbstractTestCase, buildOwner } from 'internal-test-helpers';

moduleFor(
'Ember.CoreObject',
Expand Down Expand Up @@ -62,6 +62,24 @@ moduleFor(
assert.equal(get(proxy, 'lolol'), true, 'should be able to get data from a proxy');
}

['@test should not trigger proxy assertion when retrieving a re-registered proxy (GH#16610)'](
assert
) {
let owner = buildOwner();

let someProxyishThing = CoreObject.extend({
unknownProperty() {
return true;
},
}).create();

// emulates ember-engines's process of registering services provided
// by the host app down to the engine
owner.register('thing:one', someProxyishThing, { instantiate: false });

assert.equal(owner.lookup('thing:one'), someProxyishThing);
}

['@test should not trigger proxy assertion when probing for a "symbol"'](assert) {
let proxy = CoreObject.extend({
unknownProperty() {
Expand Down