-
Notifications
You must be signed in to change notification settings - Fork 1
[WIP] Mock data source refactor and example #71
base: develop
Are you sure you want to change the base?
Conversation
I marked this PR as WIP because It's still open to discussion. I added an example of implementation that is into the app scope, not harmony, just to show how it works and how clean is the DI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Uri!
I don't see any major issue. Although I've put few comments, please check them.
Anyway, this made me think that this is somewhat related to InMemoryDataSource
; where the difference is that the MockDS would be pre-initialised by some default values (they could be randomly generated via your solution, or similar). Then, if we added pagination handling to the InMemoryDataSource
it would be so much cooler. The wonderful world of "molaria". ^_^
} | ||
|
||
protected get randUid() { | ||
return Math.random().toString(36).substr(2, 9); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this, I think you can just use this StringUtils.randomString(9)
, see: https://github.com/mobilejazz/harmony-typescript/blob/3b1c995bf777648805b710aee28b38d21abf55dd/packages/core/src/helpers/string.utils.ts
protected readonly delay = 0, | ||
) {} | ||
|
||
async get(query: Query): Promise<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, can you add explicit visibility to these methods?
async get(query: Query): Promise<T> { | |
public async get(query: Query): Promise<T> { |
value['id'] = this.randUid; | ||
} else if (typeof value['id'] === 'number') { | ||
value['id'] = this.randId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At first this fooled me and I thought you had a bug & that you were setting the methods to value['id']
, then I realised that both randId
& randUid
where using get
keyword… maybe is clearer to just have the methods and to remove get
?
value['id'] = this.randUid; | |
} else if (typeof value['id'] === 'number') { | |
value['id'] = this.randId; | |
value['id'] = this.randUid(); | |
} else if (typeof value['id'] === 'number') { | |
value['id'] = this.randId(); |
const randId = query instanceof IdQuery ? query.id : Math.floor(Math.random() * 1000); | ||
return new UserModel ( | ||
randId, | ||
`User_${randId}`, | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes more sense to call this id
:
const randId = query instanceof IdQuery ? query.id : Math.floor(Math.random() * 1000); | |
return new UserModel ( | |
randId, | |
`User_${randId}`, | |
); | |
const id = (query instanceof IdQuery) ? query.id : Math.floor(Math.random() * 1000); | |
return new UserModel(id, `User_${id}`); |
// ---> App scope | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part could be extracted to the reference… not sure where though.
Add `ngx-transalte` cache busting tip + other improvements
I updated the current MockDataSource with a version that I've been using in projects that I think it's much more versatile.