Options clobber Data? ( should be separate )

[nickperkinslondon]

coffeecup.render = (template, data = {}, options = {}) ->
data[k] = v for k, v of options
data.cache ?= off
data.stylus = require 'stylus'

These first 3 lines of "render" start brutally clobbering the "data" object by injecting the options right into the data!
This means you can never use data attributes named any of the following:
"hardcode"
"optimize"
"locals"
"cache"
"stylus"

Options should be kept separate from Data, so that there are no secret "forbidden" attribute names!

[gradus]

That is a good point. thanks for bringing this up.
I'm open to suggestions if you have any, would love to see your ideas.

On May 7, 2012, at 3:18 PM, Nick Perkins wrote:

coffeecup.render = (template, data = {}, options = {}) ->
data[k] = v for k, v of options
data.cache ?= off
data.stylus = require 'stylus'

These first 3 lines of "render" start brutally clobbering the "data" object by injecting the options right into the data!
This means you can never use data attributes named any of the following:
"hardcode"
"optimize"
"locals"
"cache"
"stylus"

Options should be kept separate from Data, so that there are no secret "forbidden" attribute names!

---

Reply to this email directly or view it on GitHub:
#29

[nickperkinslondon]

I would like to try to fix this. I am having trouble running the tests, though. Is there any documentation on exactly how to run the tests, and what modules are required? ( ie "mocha", "should",etc ? )

[nickperkinslondon]

Ok, never mind...figured it out...."cake build", "cake test"...i get it...

[gradus]

npm test should also work.

[gradus]

@nickperkinslondon it's always a good idea to look in the package.json
There's your documentation on dev dependencies and the the test script.
just npm install and npm test.
Your time working on this is much appreciated.

[gradus]

FYI the test_optimized.coffee is irrelevant.
All of those tests are covered in the mocha tests.
If you want you could run that or the optimized bench with just coffee ./optimized_bench.coffee or coffee ./text_optimized.coffee

[nickperkinslondon]

I don't think this issue can be fixed in a completely backwards-compatible way.

I have tried a fix ( keep options separate from data), but there is a problem...
...it breaks any existing code that passes Options in the Data parm.

For example, in the existing tests...the Stylus test does this:
cc.render(s, optimized: true, cache: on)

Do you see?...options are being passed as the "data" parameter.
This works fine because, currently, options are "merged" with data.
I could simply "fix" the Stylus test, but the point is that any existing code that does this will break.
Existing users whose code breaks, would have to fix their code to properly pass Options and Data separately.

I think we must either fix it once and for all ( breaking some existing code ), or don't fix it, in which case strange things will continue happen to anyone who has a data attribute name the same as an Option name, and adding new Options in the future will be, well, impossible without breaking somebody's code.

If this is not fixed, then we are left with a certain set of "reserved" words that must not be used as data attributes.
That could be awkward, especially if the data is pre-existing data, or comes from an ORM or something.

[gradus]

I have thought about these issues. that's why I wanted to see your ideas.
I'm not concerned so much with backwards compatibility.
Your point is valid and the issue should be fixed maybe for a 0.4.x release I have planned for down the road.

On May 12, 2012, at 11:47 AM, Nick Perkins wrote:

I don't think this issue can be fixed in a completely backwards-compatible way.

I have tried a fix ( keep options separate from data), but there is a problem...
...it breaks any existing code that passes Options in the Data parm.

For example, in the existing tests...the Stylus test does this:
cc.render(s, optimized: true, cache: on)

Do you see?...options are being passed as the "data" parameter.
This works fine because, currently, options are "merged" with data.
I could simply "fix" the Stylus test, but the point is that any existing code that does this will break.
Existing users whose code breaks, would have to fix their code to properly pass Options and Data separately.

I think we must either fix it once and for all ( breaking some existing code ), or don't fix it, in which case strange things will continue happen to anyone who has a data attribute name the same as an Option name, and adding new Options in the future will be, well, impossible without breaking somebody's code.

If this is not fixed, then we are left with a certain set of "reserved" words that must not be used as data attributes.
That could be awkward, especially if the data is pre-existing data, or comes from an ORM or something.

---

Reply to this email directly or view it on GitHub:
#29 (comment)

[nickperkinslondon]

I think I have the basis of a solution, which converts all options into hard-code during the compile phase.
But...there is a problem with the "stylus" option...
All of the other Options can be easily hard-coded at compile time, because their values are things like "true"/"false"
However, "stylus" is different : the user is expected to pass the module itself!
ie: options.stylus = require 'stylus'
( and if they try to use a "stylus" tag without passing the module as an option, it blows up )
This makes me think about what you said about keeping the core simple, but allowing plug-ins.
It seems that Stylus should be some sort of plug-in, not part of the core of coffeecup.
Maybe I will continue, and try to make everything else work first -- then come back to the the "stylus" question...

[nickperkinslondon]

There is also a problem with "locals":
This "option" is interpreted as a flag during compilation, triggering the "with statement".
It is also passed again to the render function because these "live links" would not survive the "baking process".
I see that all of the tests use the integrated "render" call instead of separate "compile" and "render" calls.
It's just as well -- the tests would not pass otherwise.
If Options are to be hard-coded and "baked in" to the template function, then "locals" can not be part of Options.
That means "locals" would have to be passed at "render" time, not at "compile" time.

On the whole, I have become quite concerned about the whole "baking process", where the function is serialized, munged, and re-compiled, all in the interest of magically injecting all the tag names as local variables. It is a huge hack that was done to make the template syntax absolutely minimal. The down-side, as you all know, is that you lose closures, etc, and it makes error reporting difficult.

I decided to do some experimenting, and see how far I could go with creating a similar project that does not use the "magic" injection of html tags. Instead, what if the html tags were passed to the template function as the "this" parameter? ( instead of the "data" being "this" ) On the downside, you would have to write "@H1" instead of just "h1", but on the plus-side...there is no magic. Your template is just a function, and it does not have to be stringified and re-compiled. The data can now be the parameters to the template function, which means you can have as many "data objects" as you want, and you can give them descriptive names. If you want to pass-in special helper functions or whatever, you can simply pass them in the same way as the data. If you want to provide common helpers, you just put onto the "this" object, along with all of the html tag functions.

I have up-loaded my code to a new project called "coffee-mug"
I later found that this idea was already tried six months ago by a project called "CoffeeMugg"
( lol...the name was just too obvious )

So...
I am not sure where I want to go with this.
Templates tend to be judged firstly on beauty, and secondly on speed.
Changing the template syntax from "h1" to "@H1" kind of ruins the perfect HAML-like beauty of it.
But getting rid of all magic has benefits, too...not least of which is better error reporting.

[wmertens]

@nickperkinslondon I must say I really like the approach CoffeeMugg has taken... It makes me less reticent about using it on the client side (speed). I don't mind the extra @, in fact I rather like that it marks the method as special.
(I do prefer that it uses lowercase, just like what HTML5 writing style proscribes).

This approach also allows changing the this context so that e.g. when you open an <svg> tag it only allows what is allowed in <svg>.