fix data tab not showing models #2691
Conversation
|
I would prefer to see this change being made to fix a failing test 🙈 CI passing before and after this shows that we have a gap in our testing here. Probably another thing that is blocked on having a good separated test app. |
patricklx
force-pushed
the
fix-data-tab
branch
from
da35769 to
3006117
Compare
|
@mansona I now added a failing test and ci showed also the error. then I pushed the fix |
app/services/storage.ts
Outdated
| export default class StorageService extends Service { | ||
| @service(LOCAL_STORAGE_SUPPORTED ? 'storage/local' : 'storage/memory') | ||
| declare backend: LocalStorageService | MemoryStorageService; | ||
| @tracked changed = 1; |
There was a problem hiding this comment.
@patricklx this whole system of manually updating changed to indicate something changed seems wrong to me. We should be working from derived state and use normal tracking things.
There was a problem hiding this comment.
That's to make local storage tracked
There was a problem hiding this comment.
I just saw that you have a more sophisticated implementation:
https://github.com/adopted-ember-addons/ember-tracked-local-storage/blob/master/addon/utils/tracked-local-storage.js
:)
But maybe its not really necessary?
There was a problem hiding this comment.
and we would need for both local storage and memory storage
There was a problem hiding this comment.
I just do not like manually incrementing a counter here. Perhaps you could copy over the tracked-local-storage stuff or try using something similar.
There was a problem hiding this comment.
i updated it, how about now?
| test('Model types are successfully listed and bound', async function (assert) { | ||
| respondWith('data:getModelTypes', { | ||
| type: 'data:modelTypesAdded', | ||
| modelTypes: getModelTypes(), |
There was a problem hiding this comment.
I'm unclear on how this change tests the behavior. Could you please explain?
There was a problem hiding this comment.
so, the flow was the following (before this change):
- route waits for new models
- test sendMessage with initial models
- test if models appear on page
so there is no "update" to the page after initial render
now its like this
- route waits for new models
- test sendMessage with initial EMPTY models
- waits for page to load
- sendMessage with actual models
- test if models appear on page and thus checks if update works
4 participants
Description
some computed properties are not interacting well with tracked array
fixes #2690
Screenshots