Updates for Ember 4 compatibility #194
Conversation
| @@ -63,6 +64,7 @@ export default Service.extend({ | |||
| this._alive = {}; | |||
| this._scriptLoaded = false; | |||
| this._scriptLoading = false; | |||
| this.stripeConfig = getOwner(this).lookup('config:stripe'); | |||
There was a problem hiding this comment.
What's this for? I don't see it being used
There was a problem hiding this comment.
Without that, the configs that come from the implementing platform (admin) do not load.
Can you help me understand why?
There was a problem hiding this comment.
@vladucu The idea behind this change was inspired by this change of yours. Here is the deprecation doc I based the change on:
[...] In cases where it is not possible to convert a custom injection type into a service, the value can be accessed by looking it up directly on the container instead using the lookup method.
From what I understand now, getOwner(this).lookup is a general way of accessing registered objects within an Ember application, allowing you to retrieve not just services but any type of object registered with the application's container, which is the case for this addon
There was a problem hiding this comment.
Got it. Thanks!
If this is the case, let's please also remove this line of code that might be confusing in the future.
PS Sorry for all the extra questions. I didn't realise that this was related to implicit injection deprecation from Ember 4 because there was no change in here yo outline that. In general, it's helpful to provide answers to the WHY (ex: we need this because in Ember 4 implicit injections are no longer supported) instead of WHAT (ex: this no longer works - one can tell from reading the PR considering the change was intentional)
| const application = arguments[1] || arguments[0]; | ||
| const { stripe = {} } = config; | ||
|
|
||
| application.register('config:stripe', stripe, { instantiate: false }); |
There was a problem hiding this comment.
Will this still work if we remove application.register completely?
If we want to remove completely, we should remove the initializer completely and probably just read from config/environment in the service as at the top of this file.
There was a problem hiding this comment.
Done! I tested it and it worked!
There was a problem hiding this comment.
Can I ask how did you test this pls?
I would assume that without this stripeConfig on the service will be undefined resulting in an error because the API key is missing.
There was a problem hiding this comment.
You are 100% right. I was just upgrading and downgrading plans, I need to do something like updating the credit card to really call the service. Thanks for pointing this out. I have restored the file, and it's now working.
|
✅ Linked to Task AS-407 · Upgrade to latest Ember 4.x |
This PR addresses the required changes needed for this addon to work with Ember 4