Closed kaue closed 6 years ago
I think this pull request shows promise.... However, you are complete right in its awkwardness to request a user to pass back a specific formatted array of objects.
Let me sit on this and absorb some more
I fixed and updated everything to shut up the cli testers. Except for one item related to bitHound that I can't figure out.
Here is what I came up with:
handleCustoms
- [{type:constructor, each:(value, index, parent)=>any}]
Use this to define an array of constructors to match by to convert by the each function (see example)Define types by constructors and what function to run when that type is encountered
var jsonexport = require('jsonexport');
//definitions to type cast
var customs = [{
type:Buffer,
each:function(value,index,parent){
return value.toString()
}
}]
//data
var contacts = {
'a' : Buffer.from('a2b', 'utf8'),
'b' : Buffer.from('other field', 'utf8')
};
var options={
handleCustoms:customs
}
jsonexport(contacts, options, function(err, csv){
console.log( csv )
});
@AckerApple I like this implementation a lot! but i think we should use an object instead of an array and we should cast Buffers to string by default, i think thats what most users from this module will expect.
What you think about this implementation:
jsonexport(data, {
typeHandler: {
Buffer: (value,index,parent) => value.toString(),
}
}, (err, csv) => {
});
I think its better this way because we make sure there is no duplicate handlers for the same type and its easier to extend the default handlers.
To support this we probably need to use instanceof
, since typeof Buffer.from()
returns object
.
So we would need to loop in all props from typeHandler
and check if there is one that matches.
Buffer.from('a2b', 'utf8') instanceof global['Buffer']
https://github.com/kauegimenes/jsonexport/pull/49/files#diff-d765e64fa489e3c85242992214ffb983R42
Ok, I like that too. Even simpler as an object instead on an array.
I’m using constructor to type check. I think instance of allow for inheritance checking as well. So I’m all good here.
I will work on it tomorrow. Passing out right now
@AckerApple Great! I think this new typeHandler option can replace all the other handlers. The default value for this option could be something like this:
{
typeHandler: {
String: this._handleString,
Number: this._handleNumber,
Boolean: this._handleBoolean,
Date: this._handleDate,
Buffer: this._handleBuffer, // .toString()
}
}
Then we just use Object.assign
to accept custom handlers.
https://github.com/kauegimenes/jsonexport/blob/1a0e8696591c0707afc22b0f9c4adcd029b98c3e/lib/parser/handler.js#L14
YES! All checks flipping' finally passed.... Good grief BitHound and I got into a fight!
Due to the deprecation of existing handlers, I have moved the package.version to 2.1.0
This is what I came up with:
Define types by constructors and what function to run when that type is matched
var jsonexport = require('jsonexport');
//data
var contacts = {
'a' : Buffer.from('a2b', 'utf8'),
'b' : Buffer.from('other field', 'utf8'),
'z' : 22
};
var options={
//definitions to type cast
typeHandlers:{
Array:function(value,index,parent){
return 'replaced-array';
},
Boolean:function(value,index,parent){
return 'replaced-boolean';
},
Number:function(value,index,parent){
return 'replaced-number';
},
String:function(value,index,parent){
return 'replaced-string';
},
Buffer:function(value,index,parent){
return value.toString()
}
}
}
jsonexport(contacts, options, function(err, csv){
console.log( csv )
});
The output would be:
a,a2b
b,other field
z,replaced-number
I wonder if we should document the following:
var options={
typeHandlers:{
Object:function(value,index,parent){
return 'EVERYTHING IS AN OBJECT';
}
}
}
It's not an error and a developer will notice the terrible results right away, anyhow.
Just thought I'd note it here
I wonder if we should consider runtime classses aka user made classes. This typeHandlers only supports global types. I might be over thinking but wanted to throw that out there.
Wow nice work @AckerApple as aways =)
Looks good to merge for me!
I think its a nice idea to add the everything is an object example to the README https://github.com/kauegimenes/jsonexport/pull/49#issuecomment-384975543
What other basic types we can handle by default? Maybe functions?
I’ll consider your last comment into another push and then will merge
Done and done. Please approve readme file addition of Contributors and then merge or I can merge but lets get this on NPM please.
New Contributors section as follows:
Nice! Just published jsonexport@2.1.0
on npm!
Thanks @AckerApple =)
Status
IN DEVELOPMENT
Description