ember-fastboot / ember-cli-fastboot

Server-side rendering for Ember.js apps
http://ember-fastboot.com/
MIT License
852 stars 160 forks source link

remove deprecated implicit injection #905

Closed amiarSlimane closed 1 year ago

amiarSlimane commented 1 year ago

Already solved here :

// ember-cli-fastboot/addon/services/fastboot.js
// this getter/setter pair is to avoid deprecation from [RFC - 680](https://github.com/emberjs/rfcs/pull/680)
  _fastbootInfo: computed({
    get() {
      if (this.__fastbootInfo) {
        return this.__fastbootInfo;
      }

      return getOwner(this).lookup('info:-fastboot');
    },
    set(_key, value) {
      this.__fastbootInfo = value;
      return value;
    }
  }),

but didn't remove the implicit injection:

// ember-cli-fastboot/packages/fastboot/src/fastboot-info.js
  instance.inject('service:fastboot', '_fastbootInfo', 'info:-fastboot');
habdelra commented 1 year ago

Fixes https://github.com/ember-fastboot/ember-cli-fastboot/issues/869. thanks!!

SergeAstapov commented 1 year ago

@habdelra @amiarSlimane was this accidentally closed?

habdelra commented 1 year ago

whoops!

gilest commented 1 year ago

What is needed to land this?

nlfurniss commented 1 year ago

What is needed to land this?

Tests passing for one thing :-P

nlfurniss commented 1 year ago

This fixes 1/6 failures

diff --git a/packages/fastboot/test/fixtures/shoebox/fastboot/fastboot-test.js b/packages/fastboot/test/fixtures/shoebox/fastboot/fastboot-test.js
index d7a497e..2571123 100644
--- a/packages/fastboot/test/fixtures/shoebox/fastboot/fastboot-test.js
+++ b/packages/fastboot/test/fixtures/shoebox/fastboot/fastboot-test.js
@@ -397,6 +397,21 @@ define('fastboot-test/services/fastboot', ['exports', 'ember'], function (export
       return typeof FastBoot !== 'undefined';
     }),

+    // this getter/setter pair is to avoid deprecation from [RFC - 680](https://github.com/emberjs/rfcs/pull/680)
+    _fastbootInfo: computed({
+      get() {
+        if (this.__fastbootInfo) {
+          return this.__fastbootInfo;
+        }
+
+        return getOwner(this).lookup('info:-fastboot');
+      },
+      set(_key, value) {
+        this.__fastbootInfo = value;
+        return value;
+      }
+    }),
+
     deferRendering: function deferRendering(promise) {
       _ember['default'].assert('deferRendering requires a promise or thennable object', typeof promise.then === 'function');
       this._fastbootInfo.deferRendering(promise);

Though I don't get why the whole service is copied over 🤔

mansona commented 1 year ago

I'm merging this because it clearly helps the situation 👍