Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Dec 5, 2019
Merged

Conversation

holyjak
Copy link
Contributor

@holyjak holyjak commented Nov 28, 2019

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

  • Rename overrides to something better (what???)
  • Use Spec to document the arguments to the extend-params-fn?
  • Document extend-params-fn in docstrings and perhaps the site docs?

@holyjak
Copy link
Contributor Author

holyjak commented Nov 28, 2019

(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
Copy link
Member

lacarmen commented Dec 2, 2019

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
Copy link
Contributor Author

holyjak commented Dec 2, 2019

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
Copy link
Member

lacarmen commented Dec 2, 2019

Sounds good :)

@holyjak
Copy link
Contributor Author

holyjak commented Dec 2, 2019

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

so that they can f.ex. display a list of tags with their counts.
@lacarmen
Copy link
Member

lacarmen commented Dec 5, 2019

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

@lacarmen
Copy link
Member

lacarmen commented Dec 5, 2019

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.

@lacarmen lacarmen merged commit b90000f into cryogen-project:master Dec 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants