-
Notifications
You must be signed in to change notification settings - Fork 167
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
Restricted API for helpers #1889
Conversation
3994846
to
6f0f345
Compare
6f0f345
to
f8cda95
Compare
assert( | ||
'To use `addTranslations()`, make sure to call `setupTest()`, `setupRenderingTest()`, or `setupApplicationTest()`.', | ||
context, | ||
); |
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.
I decided to remove assert()
, as it's unlikely that developers can run a test without setupTest()
, setupRenderingTest()
, or setupApplicationTest()
.
Note, the assert()
had provided a more user-friendly error message.
Named?: Options & { allowEmpty?: boolean }; | ||
Positional: [Value] | [Value, Options]; | ||
Named?: Options; | ||
Positional: [Value]; |
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.
Due to the use of inheritance (addon/helpers/-format-base.js
), #1633 had accidentally increased the API of {{format-*}}
helpers.
Why?
Closes #1735.
Before, the helpers had used the internal state
allowEmpty
, the user preferenceoptions.allowEmpty
, andisEmpty()
from@ember/utils
, to handle an input that isn't well-defined.isEmpty()
had made maintenance especially difficult, because it returnstrue
for many JavaScript values.Solution?
From now on, the
intl
service'sformat*()
andt()
methods form the source of truth. The helpers simply call these methods.