Explicitly import key-value-store as a service #16619
Draft
+37
−10
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
Draft
|
Thanks @wagenet, looks like CI failures are legitimate, not sure whether you have visibility there, but here is a snippet: |
|
I extracted this out of the context of other Ember upgrades so it looks like it may not work so well with 3.15. I'll investigate further. We may not want to merge this just yet. |
davidtaylorhq
added
the
ember-upgrade
Is this still true? Otherwise would be good to convert to draft. Also, you may want to rebase just so you won't have to make too many changes to this branch in the near future. |
|
@nattsw I just rebased it. If someone can approve the workflow I can see if it still fails and try to resolve it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Ember 3.26 introduces the deprecation detailed in RFC 680. Part of the implementation of this deprecation is switching the injection to a "get-only" property. However, Discourse relies on overwriting the injected value in some places which will now error due to the lack of a setter. The appropriate long-term solution here is to stop using
app.inject.In the short term, we can work around the issue by no longer injecting the keyValueStore, instead looking at each place it is used and defining it as a service there. Since the
inject/servicemethod from@ember/serviceprefixes with theservice:namespace, we also have to change how the keyValueStore is registered in the fist place.This appears to work as expected, however it is possible that it could cause issues for any Discourse plugins that rely on either the auto-injection or the specific name of
key-value-store:mainin the registry. I'm not sure if this is actually an issue in practice.