simple-json column type #1488
simple-json column type #1488
Conversation
|
I'm experiencing as I can guess abnormal test fail with sqlite on my local system. I cannot distinguish is it my fault or I pulled from master WIP with failing test. sqljs driver > autosave
1) should call autoSaveCallback on insert, update and delete
117 passing (52s)
4 pending
1 failing
1) sqljs driver > autosave should call autoSaveCallback on insert, update and delete:
AssertionError: expected 0 to equal 7
+ expected - actual
-0
+7
at Object.<anonymous> (test/functional/sqljs/auto-save.ts:55:29)
at step (build/compiled/test/functional/sqljs/auto-save.js:32:23)
at Object.next (build/compiled/test/functional/sqljs/auto-save.js:13:53)
at fulfilled (build/compiled/test/functional/sqljs/auto-save.js:4:58)
at <anonymous>edit: well, since CI tests have passed I can assume that's problem with my sqlite or something |
| @@ -174,6 +174,14 @@ export class DateUtils { | |||
| return value; | |||
| } | |||
|
|
|||
| static simpleJsonToString(value: any): string { | |||
There was a problem hiding this comment.
@19majkel94 I looked at simpleArrayToString, discovered that it was odd for it to be in DateUtils, but well, my PR is as simple as it gets, so I did the same thing 😢
There was a problem hiding this comment.
Ok, so we will leave it for now and later it would be refactored 😉
|
Looks like a lot of copy-paste - isn't there any generic driver when we could implement BTW, I think that it would be nice if we support subentities in this case - using class type instead of inline object type. So any additional props not mapped to the class field would be stripped. This is how it work with Mongo, right? 😕 |
|
@19majkel94 yes, copy-paste because it kinda copies functionality of simple-array. JSON.stringify and JSON.parse and that's all. What do you mean by subentities? It's just a simple field for json, no? |
|
@idchlife Some kind of embedded entities: Of course it wouldn't create columns like the normal one - the use case is just for stripping of additional props doesn't matching to the schema like with document databases and ODMs. |
|
@19majkel94 ah, you mean this is basically an embedded entity but simplified. And also JSON.stringify and JSON.parse can be used with embedded entities, if database supports json - it uses json type, if not - "text"-y type, yes? It will be a nice addition I agree with you, but simple-json is like for any kind of data you want to save in json. In my case I tried to save data about position (x, y). It seems simple with embedded entities, but I've got sqlite for local usage and postgres for backend usage. I will be using same TS types but in postgres I will have not appended columns as in embedded entities, but jsonb. And it's convenient to have json in jsonb/text and not additional columns. I think simple-json is good as it is right now. |
can you please check if your local |
| @@ -300,6 +300,34 @@ just like you stored them. | |||
|
|
|||
| Note you **MUST NOT** have any comma in values you write. | |||
|
|
|||
| ### `simple-json` column type | |||
There was a problem hiding this comment.
don't forget to put quick link in the header to a new sectionl.
|
@idchlife PR looks good, thank you very much. Please add changelog for the next 0.1.x version as well and this PR can be merged I believe. |
|
@pleerock updated PR. about sqljs - ormconfig.json is exact copy of ormconfig.json.dist (well, without comma at the end, because of comma in ormconfig.json.dist at the end it's hard to understand what's wrong at the first, if just copy-rename file, error in tests is that ormconfig.json does not exist but actually there comma which breaks json), so I don't quiet understand what's the problem :( |
|
try to remove |
|
I have merged this PR, thank you for contribution! |
No description provided.