Patch to implement feature, exporting notes to JSON (#912, issues/912). #927
Conversation
| } | ||
|
|
||
| async processItem(ItemClass, item) { | ||
| const obj = await this.buildPlainObjectForJson_(ItemClass, item); |
There was a problem hiding this comment.
I would have expected that simply calling JSON.stringfy(item) woudl be enough. Any reason this buildPlainObjectFromJson function is needed? Are there keys in item that should not be serialized? Note that we also need to export type_ or else it's not possible to know the object type.
There was a problem hiding this comment.
stringify(item) could work, but I think it's less fragile to have more control over what is written to disk, in case future code ever changes the structure of 'item'. I can simplify the method a bit though by creating a simpler object. Yes, the export does include type_, and it's verified by the test.
There was a problem hiding this comment.
stringify(item) could work, but I think it's less fragile to have more control over what is written to disk, in case future code ever changes the structure of 'item'.
I see what you mean and in some cases it would make sense to follow your approach. However in this case, the structure of the items is quite fixed - there might be new properties added but that will need to be serialised too, and on the other hand there won't be anything unusual added to it or any special properties (like type_) so it's safe to simply serialise it with JSON.stringfy and save it to disk. The item is basically exactly a row in the database table, except for the type_ property.
|
|
||
| async processItem(ItemClass, item) { | ||
| const obj = await this.buildPlainObjectForJson_(ItemClass, item); | ||
| const fileName = this.getNameWithDifferentExtension_(ItemClass, item, ".json"); |
There was a problem hiding this comment.
Instead of having a specialised function here, I think it would be better to change the systemPath method to this:
static systemPath(itemOrId, extension = null) {
if (extension === null) extension = 'md'
if (typeof itemOrId === 'string') return itemOrId + '.' + extension;
return itemOrId.id + '.' + extension;
}
Then here you can simply do const filename = ItemClass.systemPath(item, 'json');
|
Submitted new changes. I see that "AppVeyor build failed", but don't think it is related to my changes, I haven't touched any package.json files. |
|
All looks good, thanks a lot @downpoured! |
Implementing for #912.