Make require(tag_file); return "somehing"
jhthorsen opened this issue · 23 comments
I wonder if this would make sense in riotify:
var riot = require('riot');
require('./todo.tag').mount('todo');Or maybe just mount().
Right now, require("./todo.tag") doesn't return anything, but maybe it could return the global "riot" object?
How about just returning the name of the tag?
var riot = require('riot');
var Todo = require('./todo.tag');
riot.mount(Todo);var m = content.match(/^\< ([\w\-]+)/) or something?
@jhthorsen I've write an example code :-)
https://github.com/cognitom/riotify/blob/return-name-of-tag/index.js
I'm really not sure. It's really hard to revert this if it's not right. How about returning
module.exports = { riot: riot, name: name };That way we can do both:
var res = require("my-tag.tag");
res.riot.mount(res.name);Or we could even add a function which allow us to do:
require("my-tag.tag").mount(); // mount = function(){riot.mount(name)}I'm not sure, too. My proposal is just an idea for the discussion ^^
I think there's no hurry.
I'm watching these discussions. These are related to this topic, maybe.
- Server-side rendering riot/riot#145
- improved commonjs support riot/riot#358
@speier 's proposal is like this. (timer is a function)
var riot = require('riot').install()
var timer = require('./timer.tag')
var res = riot.render(timer, { start: 10 })Thanks for mentioning, every feedback/input appreciated.
My goal is mainly server-side rendering, and not sure what would be the ideal workflow.
@cognitom: I agree. Let's not rush... When that is said: I like the timer=require("./timer.tag") idea. The great thing, is that it can be extended in any direction.
@cognitom you can follow my WIP pull request at riot/riot#368
Currently it works the following way:
// require riot and install extension handler
var riot = require('riot').install()
// require tags
var timer = require('./tag/timer.tag')
var timetable = require('./tag/timetable.tag')
// init riot's dom (simple-dom)
riot.doc('<timer></timer><timetable></timetable>')
// register tags
riot.tag(timer)
riot.tag(timetable)
// mount tags
riot.mount('*', { start: 79 })
// render output
var res = riot.render()
console.log(res)
@speier Thanks. It would be nice if it keeps consistency between in-browser, server-side and Browserify/WebPack :-)
I don't get why you need riot.tag(timer). I though the compiled .tag files already did that?
The compiled .tag files could do that, however in that case you have to be able to access riot as they call riot.tag by themselves.
So as I see now if we'd like to be able to require .tag files, we have the following two options:
var timer = require('./timer.tag')require('./timer.tag')
In the first option the compiled tag is a common js module without dependencies, in the second case it's just plain js which calls the riot.tag() function, however in this second case riot is a dependency and should be available upon require which makes testing a bit harder.
Either way, both options have pros and cons. Maybe we could have both.
At the moment I opt for the first one, just to make the server-side api similar to the client, so you have to call the riot.tag function.
Beyond those two I see another solution, we can load and compile .tag files without require, actually require.extensions is deprecated in node, it's only there for compatibility reasons. So maybe we could simple extend riot.tag function to be able to load and compile files on the server-side. In this way we could register tag files, simple by riot.tag('./timer.tag') and no need to require them.
But making var timer = require('./timer.tag') work without dependencies will break the existing API. Right now, the compiler will convert the .tag file into something like this:
riot.tag("timer", "<timer>....</timer>", function(opts) { ... });
When that is said, I would like if it worked the way you want. I like the idea that there's no globals, when working with require(), but the way it is now, is that riot need to be defined in the global/default context. I guess the compiler should rather do something like this:
module.exports = { register: function(riot) { riot.tag("timer", "<timer>....</timer>", function(opts) { ... })) } }
....
var riot = require("riot");
var myTag = require("./my.tag");
tag.register(riot);
Or even simpler (with a minor patch to riot.tag):
module.exports = [ "timer", "<timer>....</timer>", function(opts) { ... })) } ]
....
var riot = require("riot");
var myTag = require("./my.tag");
riot.tag(myTag);
(Sorry, I'm from the Perl community, where backward compatibility is very important)
I don't like to break the current API neither, hence why I introduced --cjs flag to the compiler.
At least until we figure out something better.
Aha! Clever 👍
What do you think about my module.exports = [...] idea? I think it's super simple and has the potential to be extended later on (since it's an object).
Yes agree, I would go with the module.exports option as well, the current --cjs flag instructs the compiler to generate output like this:
module.exports = {
name: 'timer',
tmpl: '<p>Seconds Elapsed: { time }</p>',
fn: function(opts) { ... }
}
Aha! I like it the object you return. It's a whole lot cleaner. I just pushed b566497 as a real example on how I would implement it (with my current idea).
If/when tag() change behavior, I would much rather have your object of course. (instead of the array)
So where do we stand now? Is there still interest in an object to be returned? Got any feedback on b566497?
With the latest PR#512 it returns the tag name as string, and it could work as follows:
var riot = require('riot')
var tag = require('timer.tag')
riot.render(tag, { start: 42 }) // <timer><p>Seconds Elapsed: 42</p></timer>
Ok. Guess I'll wait until riot/riot#512 gets merged and implement the same here.
Thanks for the update! 😄
👍