diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts index c07f0850d40..744f4da75fe 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts @@ -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, @@ -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); } @@ -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); } @@ -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( diff --git a/packages/@ember/-internals/glimmer/lib/utils/bindings.ts b/packages/@ember/-internals/glimmer/lib/utils/bindings.ts index 7dbc87b37c3..d2461f10d82 100644 --- a/packages/@ember/-internals/glimmer/lib/utils/bindings.ts +++ b/packages/@ember/-internals/glimmer/lib/utils/bindings.ts @@ -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'; @@ -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; } @@ -111,7 +106,6 @@ export const AttributeBinding = { } operations.setAttribute(attribute, reference, false, null); - // operations.addDynamicAttribute(element, attribute, reference, false); }, }; diff --git a/packages/@ember/-internals/glimmer/lib/utils/curly-component-state-bucket.ts b/packages/@ember/-internals/glimmer/lib/utils/curly-component-state-bucket.ts index fd760a4c115..b2b3ff82105 100644 --- a/packages/@ember/-internals/glimmer/lib/utils/curly-component-state-bucket.ts +++ b/packages/@ember/-internals/glimmer/lib/utils/curly-component-state-bucket.ts @@ -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 { @@ -35,7 +35,7 @@ function NOOP() {} @private */ export default class ComponentStateBucket { - public classRef: VersionedReference | null = null; + public classRef: Option> = null; public argsRevision: Revision; constructor( diff --git a/packages/@ember/-internals/glimmer/tests/integration/components/curly-components-test.js b/packages/@ember/-internals/glimmer/tests/integration/components/curly-components-test.js index 86ce77a5976..91ad5be33b3 100644 --- a/packages/@ember/-internals/glimmer/tests/integration/components/curly-components-test.js +++ b/packages/@ember/-internals/glimmer/tests/integration/components/curly-components-test.js @@ -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', @@ -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() { diff --git a/packages/@ember/-internals/views/index.d.ts b/packages/@ember/-internals/views/index.d.ts index e4431dfd81d..773e5778648 100644 --- a/packages/@ember/-internals/views/index.d.ts +++ b/packages/@ember/-internals/views/index.d.ts @@ -21,6 +21,9 @@ export const ViewMixin: any; export const ViewStateSupport: any; export const TextSupport: any; +export function getElementId(view: Opaque): Option; +export function hasElementId(view: Opaque): boolean; + export function registerView(view: Opaque): void; export function unregisterView(view: Opaque): void; diff --git a/packages/@ember/-internals/views/index.js b/packages/@ember/-internals/views/index.js index 0d12bb3ad16..480aa279eda 100644 --- a/packages/@ember/-internals/views/index.js +++ b/packages/@ember/-internals/views/index.js @@ -25,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'; diff --git a/packages/@ember/-internals/views/lib/mixins/view_support.js b/packages/@ember/-internals/views/lib/mixins/view_support.js index bb3e8d0b76a..55bce206562 100644 --- a/packages/@ember/-internals/views/lib/mixins/view_support.js +++ b/packages/@ember/-internals/views/lib/mixins/view_support.js @@ -1,5 +1,6 @@ -import { guidFor } from '@ember/-internals/utils'; import { descriptorForProperty, Mixin, nativeDescDecorator } from '@ember/-internals/metal'; +import { symbol } from '@ember/-internals/utils'; +import { getViewElement } from '@ember/-internals/views'; import { assert } from '@ember/debug'; import { hasDOM } from '@ember/-internals/browser-environment'; import { matches } from '../system/utils'; @@ -11,6 +12,17 @@ function K() { return this; } +const ELEMENT_ID = symbol('elementId'); + +export function hasElementId(view) { + let elementId = view[ELEMENT_ID]; + return elementId !== null && elementId !== undefined; +} + +export function getElementId(view) { + return view[ELEMENT_ID]; +} + let mixin = { /** A list of properties of the view to apply as attributes. If the property @@ -154,6 +166,71 @@ let mixin = { }, }), + /** + The HTML `id` of the view's element in the DOM. You can provide this + value yourself but it must be unique (just as in HTML): + + ```handlebars + {{my-component elementId="a-really-cool-id"}} + ``` + + If not manually set a default value will be provided by the framework. + + Once rendered an element's `elementId` is considered immutable and you + should never change it. If you need to compute a dynamic value for the + `elementId`, you should do this when the component or element is being + instantiated: + + ```app/components/my-component.js + import Component from '@ember/component'; + + export default Component.extend({ + init() { + this._super(...arguments); + let index = this.get('index'); + this.set('elementId', 'component-id' + index); + } + }); + ``` + + @property elementId + @type String + @public + */ + elementId: nativeDescDecorator({ + configurable: false, + enumerable: false, + get() { + let value = this[ELEMENT_ID]; + + assert( + `\`elementId\` on ${this} does not match the \`id\` attribute in DOM`, + !(value && getViewElement(this) && getViewElement(this).getAttribute('id') !== value) + ); + + if (!value) { + let element = getViewElement(this); + let id = element && element.getAttribute('id'); + + if (id) { + value = this[ELEMENT_ID] = element.getAttribute('id'); + } + } + + return value; + }, + set(value) { + assert( + `cannot change \`elementId\` on ${this} once it is set`, + this[ELEMENT_ID] === null || this[ELEMENT_ID] === undefined + ); + + assert(`cannot set \`elementId\` on ${this} after rendering`, !getViewElement(this)); + + this[ELEMENT_ID] = value ? String(value) : null; + }, + }), + /** Appends the view's element to the specified parent element. @@ -233,39 +310,6 @@ let mixin = { return this.appendTo(document.body); }, - /** - The HTML `id` of the view's element in the DOM. You can provide this - value yourself but it must be unique (just as in HTML): - - ```handlebars - {{my-component elementId="a-really-cool-id"}} - ``` - - If not manually set a default value will be provided by the framework. - - Once rendered an element's `elementId` is considered immutable and you - should never change it. If you need to compute a dynamic value for the - `elementId`, you should do this when the component or element is being - instantiated: - - ```app/components/my-component.js - import Component from '@ember/component'; - - export default Component.extend({ - init() { - this._super(...arguments); - let index = this.get('index'); - this.set('elementId', 'component-id' + index); - } - }); - ``` - - @property elementId - @type String - @public - */ - elementId: null, - /** Called when a view is going to insert an element into the DOM. @@ -396,10 +440,6 @@ let mixin = { descriptorForProperty(this, 'tagName') === undefined ); - if (!this.elementId && this.tagName !== '') { - this.elementId = guidFor(this); - } - assert('Using a custom `.render` function is no longer supported.', !this.render); }, diff --git a/packages/@ember/-internals/views/lib/views/states/in_dom.js b/packages/@ember/-internals/views/lib/views/states/in_dom.js index 50e69b4243a..e801d10b0c3 100644 --- a/packages/@ember/-internals/views/lib/views/states/in_dom.js +++ b/packages/@ember/-internals/views/lib/views/states/in_dom.js @@ -1,7 +1,4 @@ import { assign } from '@ember/polyfills'; -import { addObserver } from '@ember/-internals/metal'; -import EmberError from '@ember/error'; -import { DEBUG } from '@glimmer/env'; import hasElement from './has_element'; const inDOM = assign({}, hasElement, { @@ -9,12 +6,6 @@ const inDOM = assign({}, hasElement, { // Register the view for event handling. This hash is used by // Ember.EventDispatcher to dispatch incoming events. view.renderer.register(view); - - if (DEBUG) { - addObserver(view, 'elementId', () => { - throw new EmberError("Changing a view's elementId after creation is not allowed"); - }); - } }, exit(view) {