-
Notifications
You must be signed in to change notification settings - Fork 13
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
Renamed the helpers by dropping the cq prefix #146
Conversation
…oughout the documentation
…llisions to be fixed" This reverts commit 8df608b.
'cq-aspect-ratio': typeof AspectRatioHelper; | ||
'cq-height': typeof HeightHelper; | ||
'cq-width': typeof WidthHelper; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reminder
Remove these lines in the future pull request, which will remove {{cq-aspect-ratio}}
, {{cq-height}}
, and {{cq-width}}
helpers.
import { deprecate } from '@ember/debug'; | ||
|
||
deprecate( | ||
'The {{cq-aspect-ratio}} helper has been renamed to {{aspect-ratio}}. Please update the helper name in your template.', | ||
false, | ||
{ | ||
for: 'ember-container-query', | ||
id: 'ember-container-query.rename-cq-aspect-ratio-helper', | ||
since: { | ||
available: '3.2.0', | ||
enabled: '3.2.0', | ||
}, | ||
until: '4.0.0', | ||
url: 'https://github.com/ijlee2/ember-container-query/tree/3.2.0#api', | ||
} | ||
); | ||
|
||
export { default } from 'ember-container-query/helpers/aspect-ratio'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reminder
Remove these lines in the future pull request, which will remove {{cq-aspect-ratio}}
, {{cq-height}}
, and {{cq-width}}
helpers.
import { deprecate } from '@ember/debug'; | ||
|
||
deprecate( | ||
'The {{cq-height}} helper has been renamed to {{height}}. Please update the helper name in your template.', | ||
false, | ||
{ | ||
for: 'ember-container-query', | ||
id: 'ember-container-query.rename-cq-height-helper', | ||
since: { | ||
available: '3.2.0', | ||
enabled: '3.2.0', | ||
}, | ||
until: '4.0.0', | ||
url: 'https://github.com/ijlee2/ember-container-query/tree/3.2.0#api', | ||
} | ||
); | ||
|
||
export { default } from 'ember-container-query/helpers/height'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reminder
Remove these lines in the future pull request, which will remove {{cq-aspect-ratio}}
, {{cq-height}}
, and {{cq-width}}
helpers.
import { deprecate } from '@ember/debug'; | ||
|
||
deprecate( | ||
'The {{cq-width}} helper has been renamed to {{width}}. Please update the helper name in your template.', | ||
false, | ||
{ | ||
for: 'ember-container-query', | ||
id: 'ember-container-query.rename-cq-width-helper', | ||
since: { | ||
available: '3.2.0', | ||
enabled: '3.2.0', | ||
}, | ||
until: '4.0.0', | ||
url: 'https://github.com/ijlee2/ember-container-query/tree/3.2.0#api', | ||
} | ||
); | ||
|
||
export { default } from 'ember-container-query/helpers/width'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reminder
Remove these lines in the future pull request, which will remove {{cq-aspect-ratio}}
, {{cq-height}}
, and {{cq-width}}
helpers.
import { setupRenderingTest } from 'dummy/tests/helpers'; | ||
import { hbs } from 'ember-cli-htmlbars'; | ||
import { module, test } from 'qunit'; | ||
|
||
module('Integration | Helper | cq-aspect-ratio', function (hooks) { | ||
setupRenderingTest(hooks); | ||
|
||
test('can return a hash with default values', async function (this: TestContext, assert) { | ||
test('provides a deprecation message', async function (this: TestContext, assert) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reminder
Delete this file in the future pull request, which will remove {{cq-aspect-ratio}}
, {{cq-height}}
, and {{cq-width}}
helpers.
import { setupRenderingTest } from 'dummy/tests/helpers'; | ||
import { hbs } from 'ember-cli-htmlbars'; | ||
import { module, test } from 'qunit'; | ||
|
||
module('Integration | Helper | cq-height', function (hooks) { | ||
setupRenderingTest(hooks); | ||
|
||
test('can return a hash with default values', async function (this: TestContext, assert) { | ||
test('provides a deprecation message', async function (this: TestContext, assert) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reminder
Delete this file in the future pull request, which will remove {{cq-aspect-ratio}}
, {{cq-height}}
, and {{cq-width}}
helpers.
import { setupRenderingTest } from 'dummy/tests/helpers'; | ||
import { hbs } from 'ember-cli-htmlbars'; | ||
import { module, test } from 'qunit'; | ||
|
||
module('Integration | Helper | cq-width', function (hooks) { | ||
setupRenderingTest(hooks); | ||
|
||
test('can return a hash with default values', async function (this: TestContext, assert) { | ||
test('provides a deprecation message', async function (this: TestContext, assert) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reminder
Delete this file in the future pull request, which will remove {{cq-aspect-ratio}}
, {{cq-height}}
, and {{cq-width}}
helpers.
Description
In light of strict mode template support (#130), I decided to rename the helpers by dropping the
cq
prefix. This lets us (1) minimize the syntax difference (e.g. in documentation) and (2) write templates that are easier to read and understand:One downside is, the names
height
andwidth
are less unique. More search results (e.g. from a whole string match) will be produced and it is up to the end-developer to sift the results.To be clear, this pull request does not introduce a breaking change. The old helpers (
{{cq-aspect-ratio}}
,{{cq-height}}
and{{cq-width}}
) continue to be available, but now produce a deprecation message. They will be removed in thev4.0.0
release.Migration strategy for end-developers
The helpers have been renamed by dropping the
cq
prefix. As a result, you can use your text editor's find-and-replace-all to update the helper names.cq-aspect-ratio
aspect-ratio
cq-height
height
cq-width
width
The three steps are demonstrated in commits 3-5 of this pull request.
How to fix name collisions
Scenario 1
You may be using the names
height
andwidth
for arguments or properties of a component. If so, follow the Octane syntax and clearly mark them as arguments (@
) or properties (this.
). Linters (e.g.ember-template-lint
,eslint-plugin-ember
, andglint
) can help you fix these name collisions.Here is an extreme example:
Scenario 2
When you have another addon that provides helpers named
{{aspect-ratio}}
,{{height}}
, and{{width}}
, you will need to decide which addon gets to keep the name.This scenario is unlikely according to
ember-observer
:{{aspect-ratio}}
- no matches{{height}}
- 1 match (false positive){{width}}
- 1 match (in a demo app)