Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor StorageService #1095

Merged
merged 1 commit into from
Dec 14, 2019
Merged

Refactor StorageService #1095

merged 1 commit into from
Dec 14, 2019

Conversation

chancancode
Copy link
Member

Ensure we are always using the in-memory storage service backend in tests, so that we aren't leaking test states into local storage, or otherwise change test behaviors due to existing items there.

Copy link
Member

@RobbieTheWagner RobbieTheWagner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems okay to me. I wasn't really aware of any of this stuff, so fi you say it works, good enough for me 👍

@chancancode
Copy link
Member Author

@rwwagner90 The primary difference is that it used to be that everywhere we want to inject the service, we need to be careful to inject the "right" backend, but now there is an extra indirection (the "storage" service) to select the "right" backend once, then everywhere else we can just use it as usual. I also fixed some minor bugs regarding how null and undefined are handled, and making sure we still roundtrip the objects in the in-memory store, to ensure we don't accidentally rely on them being the stored objects being the same instance or trying to store unserializable objects.

@chancancode
Copy link
Member Author

Not sure why the linter wasn't catching it before and suddenly start to care, but it flagged a few "no-new-mixins" violations 🤔

Ensure we are always using the in-memory storage service backend in
tests, so that we aren't leaking test states into local storage, or
otherwise change test behaviors due to existing items there.
@RobbieTheWagner
Copy link
Member

@chancancode looks like this is passing now. Should we merge?

@chancancode chancancode merged commit 7712164 into master Dec 14, 2019
@delete-merged-branch delete-merged-branch bot deleted the refactor-storage-service branch December 14, 2019 22:04
@chancancode
Copy link
Member Author

Yes 😄

nummi pushed a commit to nummi/ember-inspector that referenced this pull request Apr 1, 2020
Ensure we are always using the in-memory storage service backend in
tests, so that we aren't leaking test states into local storage, or
otherwise change test behaviors due to existing items there.
nummi pushed a commit to nummi/ember-inspector that referenced this pull request Apr 5, 2020
Ensure we are always using the in-memory storage service backend in
tests, so that we aren't leaking test states into local storage, or
otherwise change test behaviors due to existing items there.
patricklx pushed a commit to patricklx/ember-inspector that referenced this pull request Sep 19, 2022
Ensure we are always using the in-memory storage service backend in
tests, so that we aren't leaking test states into local storage, or
otherwise change test behaviors due to existing items there.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants