fastboot compatibility #67
Conversation
includes getOwner polypill for older versions of ember and adds mixin to insert fastboot services
| import computed from 'ember-computed'; | ||
|
|
||
| export default Mixin.create({ | ||
| _fastboot: computed(function() { |
There was a problem hiding this comment.
Correcting.
| @@ -38,6 +40,7 @@ export default Service.extend(Evented, { | |||
| this.get('scrollActivity').on('scroll', this, this._handleScroll); | |||
| } else if (this._eventsListened.indexOf(eventName) === -1) { | |||
| this._eventsListened.push(eventName); | |||
| if (this.get('_isFastBoot')) { return; } | |||
There was a problem hiding this comment.
We should probably move this check higher...I'm not sure it makes sense for us to mark that an event is being listened to (via push) if we're not actually going to call addEventListener
There was a problem hiding this comment.
Before push? Or on entering the function?
There was a problem hiding this comment.
Before push seems fine, since the scrollActivity service has its own code to handle fastboot compat
|
|
||
| module('Unit | Mixin | fastboot compat'); | ||
|
|
||
| // Replace this with your real tests. |
There was a problem hiding this comment.
Can you add a test here that just registers a mock fastboot service, and another one that doesn't (and confirms that isFastBoot is false as a result)>
There was a problem hiding this comment.
I actually wanted to ask this question originally. But yes I will certainly do that. Thanks for the review!
There was a problem hiding this comment.
The fastboot service is only available when it is injected by the fastboot server, so this.get('this.get('_isFastBoot') will be undefined, ( the computed property _fastboot will return undefined) in the browser context. To verify the existence of the fastboot service a test would need to be written that gets run as a result of being contained within a fastboot server instance, like using https://github.com/kaliber5/ember-fastboot-addon-tests .
There was a problem hiding this comment.
You don't need the real fastboot service, you can simply inject a mock service called fastboot that has an _isFastBoot property. For the non-FastBoot test, you can literally just do nothing and check that the CP evaluates to false (undefined) as expected
|
A few minor comments, but I really like where this ended up! We should be good to merge once the above is addressed. |
|
Awesome, this looks great! Thanks @tylerturdenpants! |
No description provided.