WIP: fix Ember.computed.bool is not a function

[r0one]

When using the moment-format helper on an ember app transpiled for the latest 2 versions of Chrome and Firefox (tested on Firefox 90.0.2), I get the following error: Uncaught (in promise) TypeError: Ember.computed.bool is not a function (stack trace below).

This seems to be caused by this transpiled code:

  exports.default = Ember.Helper.extend({
    moment: Ember.inject.service(),
    disableInterval: false,
    globalAllowEmpty: Ember.computed.bool('moment.__config__.allowEmpty'),
    supportsGlobalAllowEmpty: true,
    localeOrTimeZoneChanged: Ember.observer('moment.locale', 'moment.timeZone', function () {
      this.recompute();
    }),

The source code is very similar, I believe that computed.bool has been deprecated and that computed is now a decorator.

Uncaught (in promise) TypeError: Ember.computed.bool is not a function
    <anonymous> Ember
    exports loader.js:106
    _reify loader.js:143
    reify loader.js:130
    exports loader.js:104
    _reify loader.js:143
    reify loader.js:130
    exports loader.js:104
    requireModule loader.js:27
    Ember 10
    resolveComponentOrHelper opcode-compiler.js:381
    encodeOp opcode-compiler.js:2636
    pushOp opcode-compiler.js:2545
    <anonymous> opcode-compiler.js:2231
    compile opcode-compiler.js:519
    compileStatements opcode-compiler.js:2549
    maybeCompile opcode-compiler.js:2528
    compile opcode-compiler.js:2508
    compile runtime.js:6020
    <anonymous> runtime.js:2410
    evaluate runtime.js:1284
    evaluateSyscall runtime.js:5177
    evaluateInner runtime.js:5133
    evaluateOuter runtime.js:5125
    next runtime.js:6257
    _execute runtime.js:6241
    execute runtime.js:6211
    handleException runtime.js:5315
    handleException runtime.js:5551
    throw runtime.js:5246
    evaluate runtime.js:2538
    _execute runtime.js:5229
    execute runtime.js:5199
    runInTrackingTransaction validator.js:154
    execute runtime.js:5199
    rerender runtime.js:5579
    Ember 3
    inTransaction runtime.js:5019
    Ember 3
    invoke backburner.js:338
    flush backburner.js:229
    flush backburner.js:426
    _end backburner.js:960
    _boundAutorunEnd backburner.js:629
    promise callback*buildNext/< backburner.js:28
    flush Ember
    _scheduleAutorun backburner.js:1179
    _end backburner.js:970
    _boundAutorunEnd backburner.js:629
    promise callback*buildNext/< backburner.js:28
    flush Ember
    _scheduleAutorun backburner.js:1179
    _end backburner.js:970
    _boundAutorunEnd backburner.js:629
    promise callback*buildNext/< backburner.js:28
    flush Ember
    _scheduleAutorun backburner.js:1179
    _ensureInstance backburner.js:1167
    schedule backburner.js:776
    <anonymous> Ember
    fulfill rsvp.js:428
    resolve$1 rsvp.js:403
    initializePromise rsvp.js:526
    Ember 2
    initializePromise rsvp.js:520
    Promise rsvp.js:1021
    Ember 5
    invokeCallback rsvp.js:493
    publish rsvp.js:476
    <anonymous> Ember
    invoke backburner.js:338
    flush backburner.js:229
    flush backburner.js:426
    _end backburner.js:960
    _boundAutorunEnd backburner.js:629
    promise callback*buildNext/< backburner.js:28
    flush Ember
    _scheduleAutorun backburner.js:1179
    _ensureInstance backburner.js:1167

[timmyomahony]

(Edited) FYI, this pull request fixes the issue for me. For anyone needing this PR, you can install using yarn add stefanpenner/ember-moment#351/head while waiting for a new release

[r0one]

Oh, nice.

[timmyomahony]

@R0one I'm not a maintainer on this package, so I haven't merged your PR, I'm just suggesting how to install it while the maintainers create a new release. I think you should keep this PR open

[r0one]

Well, I haven't dug Ember's documentation, but according to folks from the other PR, importing bool seemed more idiomatic?

Btw I haven't ran any tests... The other PR seemed to have been taken much more seriously haha

But you're right, I'll keep it open and in WIP status so that it gets the maintainers' attention, they'll decide what to do with it.

[NullVoxPopuli]

I did some digging into Ember's computed, and it seems like computed.bool still works. So something is going wrong here by the time ember-moment is running. (result is the same with the old syntax, too)

❯ npm list ember-source | unme
my-app@0.0.0 /✂️/my-app
└── ember-source@3.27.5 

[NullVoxPopuli]

So, I think I'm in favor of merging this and getting a release out as this is a blocker to any app using ember-moment upgrading to 3.27+.

Though, I would like to know what has happened to Ember.computed in ember-moment.

Further observations about the difference between direct computed.bool usage vs ember-moment: In the same app, I reproduce via:

ember install ember-moment

add

{{moment-format (now) 'YYYY'}}

which looks like:

So it looks like ember-moment is being transpiled with the ember-global, rather than retaining the computed import. :thinking:

[NullVoxPopuli]

I think that is because ember-moment is on a very old version of ember-cli-babel.

[NullVoxPopuli]

@R0one if you want to rebase, that'd be great -- I setup GitHub Actions, so we have CI now -- we can make sure this doesn't break anything

[esbanarango]

👀

[tben]

Wouldn't this pull request be considered a breaking change? Since the code is no longer converting whatever value is set into a Boolean. Wouldn't the solution be to do the same thing that computer.bool does and wrap "Boolean()" around the fetched value.

https://github.com/emberjs/ember.js/blob/c37df4dadf49920cc70a6a1811dd9fb7a955a6e4/packages/%40ember/object/lib/computed/computed_macros.js#L389

[NullVoxPopuli]

I think the next release we do of ember-moment is going to be a breaking change anyway, because we can't test against 3.13+ atm

[esbanarango]

If anyone is blocked to upgrade ember-source from this. You can use this patch:

patches/ember-moment+8.0.1.patch

diff --git a/node_modules/ember-moment/addon/helpers/-base.js b/node_modules/ember-moment/addon/helpers/-base.js
index b3fbb05..e93671a 100644
--- a/node_modules/ember-moment/addon/helpers/-base.js
+++ b/node_modules/ember-moment/addon/helpers/-base.js
@@ -6,7 +6,8 @@ import { inject as service } from '@ember/service';
 export default Helper.extend({
   moment: service(),
   disableInterval: false,
-  globalAllowEmpty: computed.bool('moment.__config__.allowEmpty'),
+  globalAllowEmpty: computed('moment.__config__.allowEmpty', function()
+    {return this.get('moment.__config__.allowEmpty'); }),
   supportsGlobalAllowEmpty: true,
   localeOrTimeZoneChanged: observer('moment.locale', 'moment.timeZone', function() {
     this.recompute();

[NullVoxPopuli]

thanks for the stop-gap @esbanarango -- I think since it's been 8 days since asking for a rebase, I'm going to take this over. I'll post back here when released (if CI for existing support matrix is green)

[NullVoxPopuli]

superseded by https://github.com/adopted-ember-addons/ember-moment/pull/359

[NullVoxPopuli]

Fix released in 8.0.2