jsreport / jsreport-core

The minimalist jsreport rendering core
GNU Lesser General Public License v3.0
85 stars 24 forks source link

API review #1

Closed pofider closed 8 years ago

pofider commented 8 years ago

I have slightly updated the readme of jsreport core and described the API.

var jsreport = require('jsreport-core')()
// alternatively manual use of extensions turns of auto discovery
jsreport.use(require('jsreport-phantom-pdf')())
jsreport.use(require('jsreport-jsrender')())
/* -- */ 

jsreport.init().then(function () {     
   jsreport.render({ 
       template: { 
           content: '<h1>Hello {{:foo}}</h1>', 
           engine: 'jsrender', 
           recipe: 'phantom-pdf'
        }, 
        data: { 
            foo: "world"
        }
    }).then(function(resp) {
     //prints pdf with headline Hello world
     console.log(resp.content.toString())
   });
}).catch(function(e) {
  console.log(e)
})

I think that now jsreport-core can be nicely used in node.js applications (instead of toner) and together with other extensions it can be also distributed as whole jsreport.

@bjrmatos - Do you have any comments, ideas or improvements about jsreport-core api?

When we agree on the final version, I will merge toner sources into jsreport core and update articles to instruct readers how to use jsreport-core in nodejs instead of toner.

bjrmatos commented 8 years ago

looks very clean to me :+1:

in toner the API for registering an extension is:

toner.engine("jsrender", require("toner-jsrender"));
toner.recipe("wkhtmltopdf", require("toner-wkhtmltopdf")());

is very clear what key/name to use in jsreport.render, do you think is a good idea to document (in each extension) the name that each extension register in jsreport?

bjrmatos commented 8 years ago

for example, mention in the jsreport-phantom-pdf documentation that its name is phantom-pdf

pofider commented 8 years ago

You are right. I was thinking about it as well.

It should be in every extensions' docs. And also it should be mentioned in this doc it is convention to name the repository with jsreport- but the recipe/engine is just the second part.

I will improve docs in this manner.

bjrmatos commented 8 years ago

cool, i'll keep an eye on this because i would like to update jsreport-jade docs (and make sure that it works on the latest version of jsreport)

pofider commented 8 years ago

I united also jsreport-core initialization now and removed bootstrapper.

Only one approach now to initialize is:

var jsreport = require('jsreport-core')({options})
jsreport.init()

This applies only configuration passed as options and configuration passed directly to extensions. If you want to apply configuration file / env variables / command line args. You should set loadConfig:true in the options.

var jsreport = require('jsreport-core')({loadConfig: true})
jsreport.init()
//jsreport is now initialized based on [prod|dev].config

If there is nothing more to change...

I will start to update extensions to support the new manual use in jsreport-core. Publish the new version and update articles.

bjrmatos commented 8 years ago

a good change!, i think the jsreport-core API is done

pofider commented 8 years ago

Changes released in 0.2