Closed andreasblueher closed 7 years ago
This is good stuff! We finally have some visibility on the most useful properties - you already pushed them into the definitions, great job.
My colleague is working on changing CSOM handlers aswell, but that will be part of another PR most likely.
I would like to improve generators and complete validators for ResultScriptWebpart aswell, but I need some hints from you. Do you feel having a class DataProviderJson in SPMeta is a good idea? Having such a class would allow to define those settings just like you define other definitions, plus you could automatically serialize/deserialize them when needed in the handlers.
How do you handle complex (json) properties for other elements? Webparts seem to have sometimes more than one json properties which contains sub properties. Looking forward to your ideas.
CSOM might be seriously limited by the API itself, that's up to you. It would be all about crafting correct XML handling CDATA and serialization correctly. Checkout what is done for ContentEditorWebPart, there is a helper to write CDATA and v2/v3 web part props in a consistent manner.
DataProviderJson is an awesome idea. Having something with intellisense support is always great. We do that for some bits, and other bits aren't covered at all. Go ahead, with DataProviderJson, let's see what can be done.
As for general JSON handling, we tend to offload that to the developers. Sometimes people already have JSON.Net library, some go with .NET dynamic, and some already have helpers since 2013/2010, from the old projects. Hence, unless it is a new company and a new project, it might be reasonable useless to introduce such helpers. Again, happy to consider such helpers, nothing wrong with it.
Thanks for pointing out ContentEditorWebpart, will look into that, do you have a website where I can see v2/v3 schema changes?
I have a few more questions regarding the right approach for DataProviderJson. Since it's a already a string property in your current version, replacing it with a custom class would be a breaking change for people already using and setting DataProviderJson manually. Therefor ResultScriptWebparts needs to remain DataProviderJSON as a string property.
My idea was to have a "DataProvider" class inside ResultScriptWebpartDefinition and I would add a new property with the same name. If DataProviderJSON property is null or string.Empty one could use DataProvider property instead of DataProviderJSON in the handler but I feel it's not obvious enough for people to understand it right away. If property description would tell you, it might still not be enough.
Finally DataProvider would need Json serialization to be able to be used correctly. I can follow your argumentation, but without something to generate a valid json string (don't want to implement it myself) DataProvider class wouldn't work.
Thanks for directing me with this.
My idea was to have a "DataProvider" class inside ResultScriptWebpartDefinition and I would add a new property with the same name. If DataProviderJSON property is null or string.Empty one could use DataProvider property instead of DataProviderJSON in the handler but I feel it's not obvious enough for people to understand it right away. If property description would tell you, it might still not be enough.
Finally DataProvider would need Json serialization to be able to be used correctly. I can follow your argumentation, but without something to generate a valid json string (don't want to implement it myself) DataProvider class wouldn't work.
Fair call, thanks @andreasblueher.
Indeed, DataProviderJSON property must remain the same due to backward compatibility. You are absolutely right here - adding a new property is the way forward with optional fallback to DataProviderJSON property if it's defined. That way we can still run old model, allow people to choose on how the want to define search related stuff. That's one of the best thing we follow in SPMeta2 - adding new stuff gradually, incrementally with full backward compatibility.
Next, one more good point on JSON serialization. First of all, that's another dependency which we don't wan to have in SPMeta2. Did you notice it does not require anything by SharePoint itself? - pure .NET stuff. Another trick is to keep it working with .NET35/SP2010 (which is kind of holding us back a bit). Nevertheless, the workaround was already done - checkout how SPMeta2Model.ToJson) serializations works. There is a service for both JSON/XML serialization which, first of all, can be replaced with your own implementation and, secondly, wotk consistently for .NET40+/SP2013+ and .NET35/SP2010. Have a look, use that service for JSON related stuff. If not enough, let us know what's missed - we'll see what can be done.
Hope that covers all your hesitations. Also, appreciate you looking into this. Pretty old stuff considering newly released C#/.NET and other exciting things out there.
Hey,
I've asked a colleague to go ahead and extend ResultScriptWebpartDefinition, handler and validator to our needs. I understand that there's a lot more work to do regarding generator and validating json property but I have hope you've done this before and can point out some classes for reference.
ResultScriptWebpart uses a lot of properties which are defined by JSON and I would like to have a class representing those properties inside the json. Since DataProviderJSONDefinition doesn't seem to be a good idea, I was wondering if you think such a class should be part of SPMeta at all.
Looking forward to discuss those changes with you and we're planning on enhancing them further down the road.