Skip to content

Commit ca4eedd

Browse files
authored
Fix inspecting Glimmer components w/ obj inspector (#1106)
Glimmer components instead dev mode assertion getters to warn about mismatches between the Ember.js vs legacy Glimmer.js APIs. Skip these properties when inspecting them to avoid tripping them.
1 parent 74a6705 commit ca4eedd

File tree

4 files changed

+98
-17
lines changed

4 files changed

+98
-17
lines changed

ember_debug/object-inspector.js

+30-13
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,28 @@ const {
1313
const { oneWay } = computed;
1414
const { backburner, join } = run;
1515

16-
let glimmer;
17-
let metal;
16+
const GlimmerComponent = (() => {
17+
try {
18+
return window.require('@glimmer/component').default;
19+
} catch(e) {
20+
// ignore, return undefined
21+
}
22+
})();
23+
24+
let GlimmerReference = null;
25+
let metal = null;
1826
let HAS_GLIMMER_TRACKING = false;
1927
try {
20-
glimmer = Ember.__loader.require('@glimmer/reference');
28+
GlimmerReference = Ember.__loader.require('@glimmer/reference');
2129
metal = Ember.__loader.require('@ember/-internals/metal');
22-
HAS_GLIMMER_TRACKING = glimmer &&
23-
glimmer.value &&
24-
glimmer.validate &&
30+
HAS_GLIMMER_TRACKING = GlimmerReference &&
31+
GlimmerReference.value &&
32+
GlimmerReference.validate &&
2533
metal &&
2634
metal.track &&
2735
metal.tagForProperty;
2836
} catch (e) {
29-
glimmer = null;
30-
metal = null;
37+
// ignore
3138
}
3239

3340
const keys = Object.keys || Ember.keys;
@@ -237,12 +244,12 @@ export default EmberObject.extend(PortMixin, {
237244
let tagInfo = tracked[item.name] || { tag: metal.tagForProperty(object, item.name), revision: 0 };
238245
if (!tagInfo.tag) return;
239246

240-
changed = !glimmer.validate(tagInfo.tag, tagInfo.revision);
247+
changed = !GlimmerReference.validate(tagInfo.tag, tagInfo.revision);
241248
if (changed) {
242249
tagInfo.tag = metal.track(() => {
243250
value = get(object, item.name);
244251
});
245-
tagInfo.revision = glimmer.value(object, item.name);
252+
tagInfo.revision = GlimmerReference.value(object, item.name);
246253
}
247254
tracked[item.name] = tagInfo;
248255
} else {
@@ -972,7 +979,7 @@ function calculateCPs(object, mixinDetails, errorsForObject, expensiveProperties
972979
item.isTracked = true;
973980
}
974981
}
975-
tagInfo.revision = glimmer.value(object, item.name);
982+
tagInfo.revision = GlimmerReference.value(object, item.name);
976983
item.dependentKeys = getTrackedDependencies(object, item.name, tagInfo.tag);
977984
} else {
978985
value = calculateCP(object, item, errorsForObject);
@@ -1106,8 +1113,7 @@ function getDebugInfo(object) {
11061113
if (object instanceof Ember.ObjectProxy && object.content) {
11071114
object = object.content;
11081115
}
1109-
// We have to bind to object here to make sure the `this` context is correct inside _debugInfo when we call it
1110-
debugInfo = objectDebugInfo.bind(object)();
1116+
debugInfo = objectDebugInfo.call(object);
11111117
}
11121118
debugInfo = debugInfo || {};
11131119
let propertyInfo = debugInfo.propertyInfo || (debugInfo.propertyInfo = {});
@@ -1132,6 +1138,17 @@ function getDebugInfo(object) {
11321138
'element',
11331139
'targetObject'
11341140
);
1141+
} else if (GlimmerComponent && object instanceof GlimmerComponent) {
1142+
// These properties don't really exist on Glimmer Components, but
1143+
// reading their values trigger a development mode assertion. The
1144+
// more correct long term fix is to make getters lazy (shows "..."
1145+
// in the UI and only computed them when requested (when the user
1146+
// clicked on the "..." in the UI).
1147+
skipProperties.push(
1148+
'bounds',
1149+
'debugName',
1150+
'element'
1151+
);
11351152
}
11361153
return debugInfo;
11371154
}

tests/ember_debug/object-inspector-test.js

+62
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,14 @@ import require from 'require';
1818
import { destroyEIApp, setupEIApp } from '../helpers/setup-destroy-ei-app';
1919
import hasEmberVersion from '@ember/test-helpers/has-ember-version';
2020

21+
const GlimmerComponent = (function() {
22+
try {
23+
return require('@glimmer/component').default;
24+
} catch(e) {
25+
// ignore, return undefined
26+
}
27+
})();
28+
2129
let EmberDebug;
2230
let port;
2331
let App;
@@ -1010,4 +1018,58 @@ module('Ember Debug - Object Inspector', function(hooks) {
10101018
]);
10111019
});
10121020
}
1021+
1022+
// @glimmer/component 1.0 doesn't seem to support 3.4, even though it has the manager API
1023+
// Assertion Failed: Could not find custom component manager 'glimmer'
1024+
if (hasEmberVersion(3, 8) && GlimmerComponent) {
1025+
test('Inspecting GlimmerComponent does not cause errors', async function(assert) {
1026+
let instance;
1027+
1028+
class FooComponent extends GlimmerComponent {
1029+
constructor(...args) {
1030+
super(...args);
1031+
instance = this;
1032+
}
1033+
1034+
get foo() {
1035+
return 'foo';
1036+
}
1037+
1038+
bar = 'bar';
1039+
1040+
baz() {
1041+
return 'baz';
1042+
}
1043+
}
1044+
1045+
this.owner.register('component:foo', FooComponent);
1046+
this.owner.register('template:simple', hbs`<Foo />`);
1047+
1048+
await visit('/simple');
1049+
1050+
assert.ok(instance instanceof FooComponent, 'an instance of FooComponent has been created');
1051+
1052+
let { details, errors } = await inspectObject(instance);
1053+
1054+
assert.ok(details, 'has details');
1055+
assert.deepEqual(errors, [], 'has no errors');
1056+
1057+
let properties = [];
1058+
1059+
for (let mixin of details) {
1060+
for (let property of mixin.properties) {
1061+
properties.push(property.name);
1062+
}
1063+
}
1064+
1065+
assert.ok(properties.indexOf('args') > -1, 'contains args');
1066+
assert.ok(properties.indexOf('foo') > -1, 'contains foo');
1067+
assert.ok(properties.indexOf('bar') > -1, 'contains bar');
1068+
assert.ok(properties.indexOf('baz') > -1, 'contains baz');
1069+
1070+
assert.ok(properties.indexOf('bounds') === -1, 'does not contain bounds');
1071+
assert.ok(properties.indexOf('element') === -1, 'does not contain element');
1072+
assert.ok(properties.indexOf('debugName') === -1, 'does not contain debugName');
1073+
});
1074+
}
10131075
});

tests/ember_debug/view-debug-test.js

+2-3
Original file line numberDiff line numberDiff line change
@@ -251,15 +251,14 @@ module('Ember Debug - View', function(hooks) {
251251

252252
test('Supports applications that don\'t have the ember-application CSS class', async function t(assert) {
253253
let name, message;
254-
let rootElement = document.body;
255254

256255
await visit('/simple');
257256

258-
assert.dom(rootElement).hasClass(
257+
assert.dom(this.element).hasClass(
259258
'ember-application',
260259
'The rootElement has the .ember-application CSS class'
261260
);
262-
rootElement.classList.remove('ember-application');
261+
this.element.classList.remove('ember-application');
263262

264263
// Restart the inspector
265264
EmberDebug.start();

tests/helpers/setup-destroy-ei-app.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ export async function setupEIApp(EmberDebug, routes) {
2222
Router.map(routes);
2323
}
2424

25-
let App = Application.create({ autoboot: false });
25+
let App = Application.create({
26+
autoboot: false,
27+
rootElement: document.getElementById('ember-testing'),
28+
});
2629
App.register('router:main', Router);
2730

2831
await setApplication(App);

0 commit comments

Comments
 (0)