cryogen-project / cryogen-core

Cryogen's core
Eclipse Public License 1.0
69 stars 62 forks source link

WIP Enable users to derive extra params from params + site data #124

Closed holyjak closed 4 years ago

holyjak commented 4 years ago

so that they can f.ex. display a list of tags with their counts.

NOTE: This is not a finished solution - it is intended as a base for discussion before I commit too much time [possibly going in the wrong direction] to it. So what do you think, @lacarmen ?

ALTERNATIVES

Instead of adding :extend-params-fn to the overrides, we could pass it as an extra param but it is complicated because we need to do it on 2 functions and would then perhaps need to support 3 arities. This seems simpler.

TODO / CONSIDER

holyjak commented 4 years ago

(BTW, I am migrating my blog from GatsbyJS, which puts all data into GraphQL (which has some strong downsides). This proposal 👆is a minimal change but if we want to get all crazy we could consider putting all the data into a datascript and make that available to user hooks (to derive new data from existing ones) and to the pages, perhaps...

lacarmen commented 4 years ago

Instead of adding :extend-params-fn to the overrides, we could pass it as an extra param

Sorry, pass it in where?

Cryogen has been pretty stable for a couple years now. Most issues I get are requests for some sort of custom functionality that relies on more data being available. So in general, I like the idea and definitely prefer it over using something heavier like datascript but perhaps it's something we could revisit. Happy to discuss ideas with you. :)

holyjak commented 4 years ago

Instead of adding :extend-params-fn to the overrides, we could pass it as an extra param

Sorry, pass it in where?

As a 2nd param to compile-assets and compile-assets-timed though if you are OK with adding a key like this, I think that is optimal for now,

So in general, I like the idea and definitely prefer it over using something heavier like datascript but perhaps it's something we could revisit. Happy to discuss ideas with you. :)

I agree! If I ever get to it I can try to make a proof of concept with DataScript to see whether it is worth it. But for now, I prefer the smallest working change.

lacarmen commented 4 years ago

Sounds good :)

holyjak commented 4 years ago

@lacarmen What do you think needs to be done so that we can merge this PR?

lacarmen commented 4 years ago

@holyjak I guess I'm okay with passing the extra param :)

lacarmen commented 4 years ago

Current implementation is fine too, I don't see a big need to change that for now :) I don't think there's anything else to be done then.