search

fikaproductions / fika-gatsby-source-cockpit

This is a Gatsby version 2.\*.\* source plugin that feeds the GraphQL tree with Cockpit Headless CMS collections and singletons data. Actually, it supports querying raw texts (and any trivial field types), Markdown, images, galleries, assets, sets, repeaters, layout(-grid)s (currently only without nested images/assets), objects, linked collections and internationalization.
GNU General Public License v3.0
23 stars 22 forks source link

Do we need the value attribute at all? #24

Open DigitalGoldfish opened 5 years ago

DigitalGoldfish commented 5 years ago

Currently every field in the graphql schema has the properties typeand value. However when using the data in gatsby the type isn't needed anymore and we could therefore just assign the value directly to the field name.

I believe the reason for the value is that we need the type to be present for postprocessing of the data (linking images, assets, ...) and then we pass the collection item directly to the createNode function. In my opinion the better approach would be to generate the node from scratch (in CollectionNodeFactory) by copying the values from the cockpit data to a new object getting rid of the superfluous typeand valuefields in the process.

To not break old gatsby-sites and keep backwards compatibility we might need an option to turn this behaviour off & on.

WhippetsAintDogs commented 5 years ago

You are right. When I first started this plugin, I didn't have any knowledge of the inner workings of Cockpit, neither of GatsbyJS (I was building it before even doing my first project with both technologies :joy:). So, the main reason for keeping all those superfluous fields was that I didn't know if they were going to be useful or not to the end user at that time.

That being said, I agree that cutting them out is a necessity, but I don't think that keeping the option for backwards compatibility is required. Since the beginning of this project, I've always bumped the 'patch' version of this package (1.0.0 → 1.0.X → 1.0.11) since we were only working on the collection aspect of Cockpit. In the future, I intend to bump minor versions for anything else like the singletons, maybe the user/asset nodes as well. I think that we could bump a major version for this change (the value/type fields removing) in order to advertise that this is going to be a breaking change.

https://docs.npmjs.com/about-semantic-versioning

DigitalGoldfish commented 5 years ago

Okay great - I admit that I did expect more pushback on this issue and that's why I proposed the backwards compatiblity but if we can omit it that's fine with me too. :)

DigitalGoldfish commented 5 years ago

I started implementing this today and it should be ready for review/testing after the weekend.

WhippetsAintDogs commented 5 years ago

Great ! Thanks @DigitalGoldfish, it's going to simplify the queries a lot :)

michaelpanik commented 4 years ago

Hey @DigitalGoldfish any update on this? I'm starting a Cockpit/Gatsby project and it would make the queries a lot simpler!

eXaminator commented 4 years ago

Just to add another view to this: The type field CAN be useful! I use it to determine if a field is a WYSIWYG field and do some processing on it, like extracting links and images to have them be handled by gatsby.

For this reason I would suggest to add a simplified object that only contains the values but also keep the old fields object with the type for more specific handling.

DigitalGoldfish commented 4 years ago

Sorry, I don't even remember what happened that stopped me from finishing this. But we mostly moved away from Cockpit. I though remember that I already had the "value"-less syntax implemented in my fork so I guess i can dig that out again ...

Here is the link: https://github.com/MangoArt/gatsby-source-cockpit

Not sure how helpful this is ...