From db9015482e3ef43989042880805b3672182a4025 Mon Sep 17 00:00:00 2001 From: Gavin Joyce Date: Thu, 18 Apr 2019 20:05:19 +0100 Subject: [PATCH] add `isVisible` deprecation warning --- .../glimmer/lib/component-managers/curly.ts | 11 ++- .../-internals/glimmer/lib/utils/bindings.ts | 66 ++++++++++++---- .../components/curly-components-test.js | 78 ++++++++++++------- packages/@ember/deprecated-features/index.ts | 1 + 4 files changed, 107 insertions(+), 49 deletions(-) diff --git a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts index 881b3cc7309..757f4185d45 100644 --- a/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts +++ b/packages/@ember/-internals/glimmer/lib/component-managers/curly.ts @@ -50,6 +50,7 @@ import ComponentStateBucket, { Component } from '../utils/curly-component-state- import { processComponentArgs } from '../utils/process-args'; import AbstractManager from './abstract'; import DefinitionState from './definition-state'; +import { EMBER_COMPONENT_IS_VISIBLE } from '@ember/deprecated-features'; function aliasIdToElementId(args: Arguments, props: any) { if (args.named.has('id')) { @@ -92,8 +93,10 @@ function applyAttributeBindings( operations.setAttribute('id', PrimitiveReference.create(id), false, null); } - if (seen.indexOf('style') === -1) { - IsVisibleBinding.install(element, component, operations); + if (EMBER_COMPONENT_IS_VISIBLE) { + if (seen.indexOf('style') === -1) { + IsVisibleBinding.install(element, component, operations); + } } } @@ -368,7 +371,9 @@ export default class CurlyComponentManager } else { let id = component.elementId ? component.elementId : guidFor(component); operations.setAttribute('id', PrimitiveReference.create(id), false, null); - IsVisibleBinding.install(element, component, operations); + if (EMBER_COMPONENT_IS_VISIBLE) { + IsVisibleBinding.install(element, component, operations); + } } if (classRef) { diff --git a/packages/@ember/-internals/glimmer/lib/utils/bindings.ts b/packages/@ember/-internals/glimmer/lib/utils/bindings.ts index 7dbc87b37c3..3e702337be5 100644 --- a/packages/@ember/-internals/glimmer/lib/utils/bindings.ts +++ b/packages/@ember/-internals/glimmer/lib/utils/bindings.ts @@ -1,5 +1,5 @@ import { get } from '@ember/-internals/metal'; -import { assert } from '@ember/debug'; +import { assert, deprecate } from '@ember/debug'; import { dasherize } from '@ember/string'; import { Opaque, Option, Simple } from '@glimmer/interfaces'; import { CachedReference, combine, map, Reference, Tag } from '@glimmer/reference'; @@ -9,6 +9,7 @@ import { ROOT_REF } from '../component'; import { Component } from './curly-component-state-bucket'; import { referenceFromParts } from './references'; import { htmlSafe, isHTMLSafe, SafeString } from './string'; +import { EMBER_COMPONENT_IS_VISIBLE } from '@ember/deprecated-features'; export function referenceForKey(component: Component, key: string) { return component[ROOT_REF].get(key); @@ -128,11 +129,24 @@ class StyleBindingReference extends CachedReference { compute(): string | SafeString { let value = this.inner.value(); - let isVisible = this.isVisible.value(); - if (isVisible !== false) { - return value; - } else if (!value) { + if (EMBER_COMPONENT_IS_VISIBLE) { + let isVisible = this.isVisible.value(); + + if (isVisible !== undefined) { + deprecate(`\`isVisible\` is deprecated (from "${this.isVisible.parentValue._debugContainerKey}")`, false, { + id: 'ember-component.is-visible', + until: '4.0.0', + url: 'https://deprecations.emberjs.com/v3.x#toc_ember-component-is-visible', + }); + } + + if (isVisible !== false) { + return value; + } + } + + if (!value) { return SAFE_DISPLAY_NONE; } else { let style = value + ' ' + DISPLAY_NONE; @@ -143,20 +157,38 @@ class StyleBindingReference extends CachedReference { export const IsVisibleBinding = { install(_element: Simple.Element, component: Component, operations: ElementOperations) { - operations.setAttribute( - 'style', - map(referenceForKey(component, 'isVisible'), this.mapStyleValue), - false, - null - ); - // // the upstream type for addDynamicAttribute's `value` argument - // // appears to be incorrect. It is currently a Reference, I - // // think it should be a Reference. - // operations.addDynamicAttribute(element, 'style', ref as any as Reference, false); + if (EMBER_COMPONENT_IS_VISIBLE) { + let componentMapStyleValue = (isVisible: boolean) => { + return this.mapStyleValue(isVisible, component); + } + + operations.setAttribute( + 'style', + map(referenceForKey(component, 'isVisible'), componentMapStyleValue), + false, + null + ); + // // the upstream type for addDynamicAttribute's `value` argument + // // appears to be incorrect. It is currently a Reference, I + // // think it should be a Reference. + // operations.addDynamicAttribute(element, 'style', ref as any as Reference, false); + } }, - mapStyleValue(isVisible: boolean) { - return isVisible === false ? SAFE_DISPLAY_NONE : null; + mapStyleValue(isVisible: boolean, component: Component) { + if (EMBER_COMPONENT_IS_VISIBLE) { + if (isVisible !== undefined) { + deprecate(`\`isVisible\` is deprecated (from "${component._debugContainerKey}")`, false, { + id: 'ember-component.is-visible', + until: '4.0.0', + url: 'https://deprecations.emberjs.com/v3.x#toc_ember-component-is-visible', + }); + } + + return isVisible === false ? SAFE_DISPLAY_NONE : null; + } else { + return undefined; + } }, }; 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 ebcc64dd76f..b69dddea30b 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 @@ -2904,22 +2904,30 @@ moduleFor( template: `

foo

`, }); - this.render(`{{foo-bar id="foo-bar" isVisible=visible}}`, { - visible: false, - }); + expectDeprecation(() => { + this.render(`{{foo-bar id="foo-bar" isVisible=visible}}`, { + visible: false, + }); + }, '`isVisible` is deprecated (from "component:foo-bar")'); assertStyle('display: none;'); this.assertStableRerender(); - runTask(() => { - set(this.context, 'visible', true); - }); + expectDeprecation(() => { + runTask(() => { + set(this.context, 'visible', true); + }); + }, '`isVisible` is deprecated (from "component:foo-bar")'); + assertStyle(''); - runTask(() => { - set(this.context, 'visible', false); - }); + expectDeprecation(() => { + runTask(() => { + set(this.context, 'visible', false); + }); + }, '`isVisible` is deprecated (from "component:foo-bar")'); + assertStyle('display: none;'); } @@ -2933,9 +2941,11 @@ moduleFor( template: `

foo

`, }); - this.render(`{{foo-bar id="foo-bar" isVisible=visible}}`, { - visible: false, - }); + expectDeprecation(() => { + this.render(`{{foo-bar id="foo-bar" isVisible=visible}}`, { + visible: false, + }); + }, '`isVisible` is deprecated (from "component:foo-bar")'); this.assertComponentElement(this.firstChild, { tagName: 'div', @@ -2944,18 +2954,22 @@ moduleFor( this.assertStableRerender(); - runTask(() => { - set(this.context, 'visible', true); - }); + expectDeprecation(() => { + runTask(() => { + set(this.context, 'visible', true); + }); + }, '`isVisible` is deprecated (from "component:foo-bar")'); this.assertComponentElement(this.firstChild, { tagName: 'div', attrs: { id: 'foo-bar', style: styles('color: blue;') }, }); - runTask(() => { - set(this.context, 'visible', false); - }); + expectDeprecation(() => { + runTask(() => { + set(this.context, 'visible', false); + }); + }, '`isVisible` is deprecated (from "component:foo-bar")'); this.assertComponentElement(this.firstChild, { tagName: 'div', @@ -2986,25 +3000,31 @@ moduleFor( template: `

foo

`, }); - this.render(`{{foo-bar id="foo-bar" foo=foo isVisible=visible}}`, { - visible: false, - foo: 'baz', - }); + expectDeprecation(() => { + this.render(`{{foo-bar id="foo-bar" foo=foo isVisible=visible}}`, { + visible: false, + foo: 'baz', + }); + }, '`isVisible` is deprecated (from "component:foo-bar")'); assertStyle('display: none;'); this.assertStableRerender(); - runTask(() => { - set(this.context, 'visible', true); - }); + expectDeprecation(() => { + runTask(() => { + set(this.context, 'visible', true); + }); + }, '`isVisible` is deprecated (from "component:foo-bar")'); assertStyle(''); - runTask(() => { - set(this.context, 'visible', false); - set(this.context, 'foo', 'woo'); - }); + expectDeprecation(() => { + runTask(() => { + set(this.context, 'visible', false); + set(this.context, 'foo', 'woo'); + }); + }, '`isVisible` is deprecated (from "component:foo-bar")'); assertStyle('display: none;'); assert.equal(this.firstChild.getAttribute('foo'), 'woo'); diff --git a/packages/@ember/deprecated-features/index.ts b/packages/@ember/deprecated-features/index.ts index 724ffb311f3..9867ac2c86b 100644 --- a/packages/@ember/deprecated-features/index.ts +++ b/packages/@ember/deprecated-features/index.ts @@ -14,3 +14,4 @@ export const ALIAS_METHOD = !!'3.9.0'; export const APP_CTRL_ROUTER_PROPS = !!'3.10.0-beta.1'; export const FUNCTION_PROTOTYPE_EXTENSIONS = !!'3.11.0-beta.1'; export const MOUSE_ENTER_LEAVE_MOVE_EVENTS = !!'3.13.0-beta.1'; +export const EMBER_COMPONENT_IS_VISIBLE = !!'3.14.0-beta.5';