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

[WIP] deprecate view registry #17804

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 10 additions & 10 deletions packages/@ember/-internals/glimmer/lib/component-managers/curly.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { privatize as P } from '@ember/-internals/container';
import { get } from '@ember/-internals/metal';
import { getOwner } from '@ember/-internals/owner';
import { guidFor } from '@ember/-internals/utils';
import {
addChildView,
getElementId,
getViewId,
hasElementId,
OwnedTemplateMeta,
setElementView,
setViewElement,
Expand Down Expand Up @@ -92,7 +94,7 @@ function applyAttributeBindings(
}

if (seen.indexOf('id') === -1) {
let id = component.elementId ? component.elementId : guidFor(component);
let id = hasElementId(component) ? getElementId(component) : getViewId(component);
operations.setAttribute('id', PrimitiveReference.create(id), false, null);
}

Expand Down Expand Up @@ -352,7 +354,7 @@ export default class CurlyComponentManager
if (attributeBindings && attributeBindings.length) {
applyAttributeBindings(element, attributeBindings, component, operations);
} else {
let id = component.elementId ? component.elementId : guidFor(component);
let id = hasElementId(component) ? getElementId(component) : getViewId(component);
operations.setAttribute('id', PrimitiveReference.create(id), false, null);
IsVisibleBinding.install(element, component, operations);
}
Expand Down Expand Up @@ -521,17 +523,15 @@ export function processComponentInitializationAssertions(component: Component, p
);

assert(
`You cannot use \`classNameBindings\` on a tag-less component: ${component}`,
component.tagName !== '' ||
!component.classNameBindings ||
component.classNameBindings.length === 0
`You cannot use \`elementId\` on a tag-less component: ${component}`,
component.tagName !== '' || !hasElementId(component) || props.id === getElementId(component)
);

assert(
`You cannot use \`elementId\` on a tag-less component: ${component}`,
`You cannot use \`classNameBindings\` on a tag-less component: ${component}`,
component.tagName !== '' ||
props.id === component.elementId ||
(!component.elementId && component.elementId !== '')
!component.classNameBindings ||
component.classNameBindings.length === 0
);

assert(
Expand Down
30 changes: 7 additions & 23 deletions packages/@ember/-internals/glimmer/lib/renderer.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ENV } from '@ember/-internals/environment';
import { runInTransaction } from '@ember/-internals/metal';
import { getViewElement, getViewId } from '@ember/-internals/views';
import { getViewElement, getViewId, registerView, unregisterView } from '@ember/-internals/views';
import { assert } from '@ember/debug';
import { backburner, getCurrentRunLoop } from '@ember/runloop';
import { Option, Simple } from '@glimmer/interfaces';
Expand Down Expand Up @@ -240,14 +240,9 @@ function loopEnd() {
backburner.on('begin', loopBegin);
backburner.on('end', loopEnd);

interface ViewRegistry {
[viewId: string]: Opaque;
}

export abstract class Renderer {
private _env: Environment;
private _rootTemplate: any;
private _viewRegistry: ViewRegistry;
private _destinedForDOM: boolean;
private _destroyed: boolean;
private _roots: RootState[];
Expand All @@ -259,13 +254,11 @@ export abstract class Renderer {
constructor(
env: Environment,
rootTemplate: OwnedTemplate,
viewRegistry: ViewRegistry,
destinedForDOM = false,
builder = clientBuilder
) {
this._env = env;
this._rootTemplate = rootTemplate;
this._viewRegistry = viewRegistry;
this._destinedForDOM = destinedForDOM;
this._destroyed = false;
this._roots = [];
Expand Down Expand Up @@ -310,17 +303,12 @@ export abstract class Renderer {
this._scheduleRevalidate();
}

register(view: any) {
let id = getViewId(view);
assert(
'Attempted to register a view with an id already in use: ' + id,
!this._viewRegistry[id]
);
this._viewRegistry[id] = view;
register(view: Opaque) {
registerView(view);
}

unregister(view: any) {
delete this._viewRegistry[getViewId(view)];
unregister(view: Opaque) {
unregisterView(view);
}

remove(view: Component) {
Expand Down Expand Up @@ -513,15 +501,13 @@ export class InertRenderer extends Renderer {
static create({
env,
rootTemplate,
_viewRegistry,
builder,
}: {
env: Environment;
rootTemplate: OwnedTemplate;
_viewRegistry: any;
builder: any;
}) {
return new this(env, rootTemplate, _viewRegistry, false, builder);
return new this(env, rootTemplate, false, builder);
}

getElement(_view: Opaque): Option<Simple.Element> {
Expand All @@ -535,15 +521,13 @@ export class InteractiveRenderer extends Renderer {
static create({
env,
rootTemplate,
_viewRegistry,
builder,
}: {
env: Environment;
rootTemplate: OwnedTemplate;
_viewRegistry: any;
builder: any;
}) {
return new this(env, rootTemplate, _viewRegistry, true, builder);
return new this(env, rootTemplate, true, builder);
}

getElement(view: Opaque): Option<Simple.Element> {
Expand Down
12 changes: 3 additions & 9 deletions packages/@ember/-internals/glimmer/lib/utils/bindings.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { get } from '@ember/-internals/metal';
import { getElementId, getViewId } from '@ember/-internals/views';
import { assert } from '@ember/debug';
import { dasherize } from '@ember/string';
import { Opaque, Option, Simple } from '@glimmer/interfaces';
Expand Down Expand Up @@ -86,13 +86,8 @@ export const AttributeBinding = {
let [prop, attribute, isSimple] = parsed;

if (attribute === 'id') {
let elementId = get(component, prop);
if (elementId === undefined || elementId === null) {
elementId = component.elementId;
}
elementId = PrimitiveReference.create(elementId);
operations.setAttribute('id', elementId, true, null);
// operations.addStaticAttribute(element, 'id', elementId);
let elementId = component[prop] || getElementId(component) || getViewId(component);
operations.setAttribute('id', PrimitiveReference.create(elementId), true, null);
return;
}

Expand All @@ -111,7 +106,6 @@ export const AttributeBinding = {
}

operations.setAttribute(attribute, reference, false, null);
// operations.addDynamicAttribute(element, attribute, reference, false);
},
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { clearElementView, clearViewElement, getViewElement } from '@ember/-internals/views';
import { Revision, VersionedReference } from '@glimmer/reference';
import { CapturedNamedArguments } from '@glimmer/runtime';
import { Opaque } from '@glimmer/util';
import { Opaque, Option } from '@glimmer/util';
import Environment from '../environment';

export interface Component {
Expand Down Expand Up @@ -35,7 +35,7 @@ function NOOP() {}
@private
*/
export default class ComponentStateBucket {
public classRef: VersionedReference<Opaque> | null = null;
public classRef: Option<VersionedReference<Opaque>> = null;
public argsRevision: Revision;

constructor(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ moduleFor(
if (EmberDev && !EmberDev.runningProdBuild) {
let willThrow = () => run(null, set, component, 'elementId', 'herpyderpy');

assert.throws(willThrow, /Changing a view's elementId after creation is not allowed/);
assert.throws(willThrow, /cannot change `elementId` on <.+> once it is set/);

this.assertComponentElement(this.firstChild, {
tagName: 'div',
Expand Down Expand Up @@ -278,6 +278,23 @@ moduleFor(
});
}

['@test elementId can not be a computed property']() {
let FooBarComponent = Component.extend({
elementId: computed(function() {
return 'foo-bar';
}),
});

this.registerComponent('foo-bar', {
ComponentClass: FooBarComponent,
template: 'hello',
});

expectAssertion(() => {
this.render('{{foo-bar}}');
}, /You cannot use a computed property for the component's `elementId` \(<.+?>\)\./);
}

['@test tagName can not be a computed property']() {
let FooBarComponent = Component.extend({
tagName: computed(function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class LifeCycleHooksTest extends RenderingTestCase {
this.components = {};
this.componentRegistry = [];
this.teardownAssertions = [];
this.viewRegistry = this.owner.lookup('-view-registry:main');
}

afterEach() {
Expand Down Expand Up @@ -65,22 +64,6 @@ class LifeCycleHooksTest extends RenderingTestCase {
};
}

assertRegisteredViews(label) {
let viewRegistry = this.viewRegistry;
let topLevelId = getViewId(this.component);
let actual = Object.keys(viewRegistry)
.sort()
.filter(id => id !== topLevelId);

if (this.isInteractive) {
let expected = this.componentRegistry.sort();

this.assert.deepEqual(actual, expected, 'registered views - ' + label);
} else {
this.assert.deepEqual(actual, [], 'no views should be registered for non-interactive mode');
}
}

registerComponent(name, { template = null }) {
let pushComponent = instance => {
this.components[name] = instance;
Expand Down Expand Up @@ -335,7 +318,6 @@ class LifeCycleHooksTest extends RenderingTestCase {
});

this.assertText('Twitter: @tomdale|Name: Tom Dale|Website: tomdale.net');
this.assertRegisteredViews('intial render');

this.assertHooks({
label: 'after initial render',
Expand Down Expand Up @@ -532,8 +514,6 @@ class LifeCycleHooksTest extends RenderingTestCase {
['the-bottom', 'willDestroy'],
],
});

this.assertRegisteredViews('after destroy');
});
}

Expand Down Expand Up @@ -575,7 +555,6 @@ class LifeCycleHooksTest extends RenderingTestCase {
);

this.assertText('Twitter: @tomdale|Name: Tom Dale|Website: tomdale.net');
this.assertRegisteredViews('intial render');

this.assertHooks({
label: 'after initial render',
Expand Down Expand Up @@ -853,8 +832,6 @@ class LifeCycleHooksTest extends RenderingTestCase {
['the-last-child', 'willDestroy'],
],
});

this.assertRegisteredViews('after destroy');
});
}

Expand Down Expand Up @@ -889,7 +866,6 @@ class LifeCycleHooksTest extends RenderingTestCase {
});

this.assertText('Top: Middle: Bottom: @tomdale');
this.assertRegisteredViews('intial render');

this.assertHooks({
label: 'after initial render',
Expand Down Expand Up @@ -1038,8 +1014,6 @@ class LifeCycleHooksTest extends RenderingTestCase {
['the-bottom', 'willDestroy'],
],
});

this.assertRegisteredViews('after destroy');
});
}

Expand Down Expand Up @@ -1074,7 +1048,6 @@ class LifeCycleHooksTest extends RenderingTestCase {
);

this.assertText('Item: 1Item: 2Item: 3Item: 4Item: 5');
this.assertRegisteredViews('intial render');

let initialHooks = () => {
let ret = [['an-item', 'init'], ['an-item', 'on(init)'], ['an-item', 'didReceiveAttrs']];
Expand Down Expand Up @@ -1261,8 +1234,6 @@ class LifeCycleHooksTest extends RenderingTestCase {

nonInteractive: [['no-items', 'willDestroy'], ['nested-item', 'willDestroy']],
});

this.assertRegisteredViews('after destroy');
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import Controller from '@ember/controller';
import {
getRootViews,
getChildViews,
getElementView,
getViewBounds,
getViewClientRects,
getViewBoundingClientRect,
Expand Down Expand Up @@ -252,9 +253,8 @@ moduleFor(
}

viewFor(id) {
let owner = this.applicationInstance;
let registry = owner.lookup('-view-registry:main');
return registry[id];
let element = this.element.querySelector(`#${id}`);
return getElementView(element);
}
}
);
Expand Down
6 changes: 6 additions & 0 deletions packages/@ember/-internals/views/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ export const ViewMixin: any;
export const ViewStateSupport: any;
export const TextSupport: any;

export function getElementId(view: Opaque): Option<string>;
export function hasElementId(view: Opaque): boolean;

export function registerView(view: Opaque): void;
export function unregisterView(view: Opaque): void;

export function getElementView(element: Simple.Element): Opaque;
export function getViewElement(view: Opaque): Option<Simple.Element>;
export function setElementView(element: Simple.Element, view: Opaque): void;
Expand Down
4 changes: 3 additions & 1 deletion packages/@ember/-internals/views/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ export { default as jQuery, jQueryDisabled } from './lib/system/jquery';
export {
addChildView,
isSimpleClick,
registerView,
unregisterView,
getViewBounds,
getViewClientRects,
getViewBoundingClientRect,
Expand All @@ -23,7 +25,7 @@ export { default as CoreView } from './lib/views/core_view';
export { default as ClassNamesSupport } from './lib/mixins/class_names_support';
export { default as ChildViewsSupport } from './lib/mixins/child_views_support';
export { default as ViewStateSupport } from './lib/mixins/view_state_support';
export { default as ViewMixin } from './lib/mixins/view_support';
export { default as ViewMixin, getElementId, hasElementId } from './lib/mixins/view_support';
export { default as ActionSupport } from './lib/mixins/action_support';
export { MUTABLE_CELL } from './lib/compat/attrs';
export { default as lookupPartial, hasPartial } from './lib/system/lookup_partial';
Expand Down
Loading