Add solution to Code Generator for properties with few types in JS #128
Comments
|
Can you elaborate here? We probably have the same requirement when generating inputs for Highcharts cloud, as well as Highcharts iOS. We have been discussing a stricter definition of the various options in the doclets, to make auto-genaration of wrappers fail-safe. For example, pie.size can be either a string or a number. If string, it should be a percentage like "50%". If number, it is pixels. |
|
There is no proper solution inside Code Generator for such properties. There is no automatic mechanism implemented inside Code Generator for properties with multiple types. |
Great, but let's discuss this first, so we get a solution that is consistent across Highcharts.NET, Highcharts Cloud, our Typescript definitions and other wrappers. Yes I suppose creating extended property names is a quick and viable way of resolving the problem. For example:
... however it would be more intuitive if the options were called But implementing that would take a bit more work. We would need to add a structure in the JSDoc doclets that defines how these settings should be resolved. And this would be settings that only apply to strictly typed environments - in the vanilla JavaScript API nothing would change. @cvasseng, as the author of the TypeScript definition and the Editor, what do you think? @pawelfus any thoghts? |
|
@brofixo Can you easily generate a list of those double-typed options so we can get an overview of what we're dealing with? I know some of them types can easily be skipped. For example, all animations can be |
|
Yes, but I'll do this at evening.
Daniel Drwięga
19.09.2017 09:06 "Torstein Hønsi" <[email protected]> napisał(a):
… @brofixo <https://github.com/brofixo> Can you easily generate a list of
those double-typed options so we can get an overview of what we're dealing
with? I know some of them types can easily be skipped. For example, all
animations can be Boolean or Object. We don't need to deal with the
boolean, instead we can disable animation by setting { duration: 0 }.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#128 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ARFTt4FApdvV9YZSJ5psA5C9b03eIAIjks5sj2f3gaJpZM4PZ7p7>
.
|
|
I can set different names of properties, if I will get (for multiple types
property) property name for each type in JSON ( for example: SizePixels,
SizePercent).
Daniel Drwięga
19.09.2017 09:03 "Torstein Hønsi" <[email protected]> napisał(a):
… I want to implement this as soon as possible.
Great, but let's discuss this first, so we get a solution that is
consistent across Highcharts.NET, Highcharts Cloud, our Typescript
definitions and other wrappers.
Yes I suppose creating extended property names is a quick and viable way
of resolving the problem. For example:
Pie.Size would be a number designating pixels, and
Pie.SizeString would be a string on the for 50%.
... however it would be more intuitive if the options were called
Pie.SizePixels and Pie.SizePercent and both options were doubles, right?
But implementing that would take a bit more work. We would need to add a
structure in the JSDoc doclets that defines how these settings should be
resolved. And this would be settings that only apply to strictly typed
environments - in the vanilla JavaScript API nothing would change.
@cvasseng <https://github.com/cvasseng>, as the author of the TypeScript
definition and the Editor, what do you think? @pawelfus
<https://github.com/pawelfus> any thoghts?
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#128 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ARFTt8Xem9QPcqRbp5RSNmDswcGjaEU7ks5sj2dZgaJpZM4PZ7p7>
.
|
|
@TorsteinHonsi: For the typescript generator, I used a preferred order on multi-type properties. Basically it would revert to a single type in this order This is somewhat of a problem though, for iOS, .NET, and TypeScript. I'm not sure what the best way of handling it is. It feels a bit messy to add even more tags to the JSDoc stuff. It might be better to maintain a "typed language" dictionary separatly, for those options with types that does not translate well to typed languages. The doc generator could then look properties up in that map for a given product. We could do that in combination with creating types for object properties (e.g. an Another problem if relying on JSDoc for iOS/.Net/TS, is that we need |
Definately. We can maintain a JSON config file, that would be a whole lot easier. In some of these cases we already have @typedef tags defined: These are already parsed and used in the class reference. We could extend on that and make it consistent. Let's wait for @brofixo's dump so we get an overview of the scope. |
|
Sounds good. We could map on primitive types as well as typedefs also. I think that's the cleanest solution (e.g. I can probably throw together a script to generate both the initial dictionary, and the rest of the typedefs too. |
|
I agree, that including more jsdoc comments in sources will become messy. Can API generator just take another |
|
That's pretty much what I was thinking, yes. :) Basically have a JSON dictionary which maps between a generated typename (e.g. Then, for iOS and .NET, all that would be required is to add the actual types in that dictionary. The dictionary could also support defining multiple types, so that e.g. |
|
Are there cases with .NET (or iOS) where the desciption for an option is different than the one in vanilla? |
|
For example
If we will have two params ( |
|
I've got from JSON such double types: plotOptions.bar.states.hover.animation : Boolean|Object |
|
Thanks @brofixo! We are continuing the discussion on Slack - did you receive the notifications? |
|
I'm only in #dotnet group.
Daniel Drwięga
20.09.2017 8:45 AM "Torstein Hønsi" <[email protected]> napisał(a):
… Thanks @brofixo <https://github.com/brofixo>! We are continuing the
discussion on Slack - did you receive the notifications?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#128 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ARFTt3u-DVtpjIFW4Yyy1Q9ORtez6lndks5skLR1gaJpZM4PZ7p7>
.
|
No description provided.
The text was updated successfully, but these errors were encountered: