Reflective discovery of styleable properties
Thiago Milczarek Sayão
thiago.sayao at gmail.com
Wed Dec 6 13:42:35 UTC 2023
Hi,
Maybe it could generate the boilerplate like Lombok does?
So it has the benefits of eliminating error-prone boilerplate code and the
benefits of not having the cost of reflection at runtime.
-- Thiago.
Em qua., 6 de dez. de 2023 às 10:28, John Hendrikx <john.hendrikx at gmail.com>
escreveu:
> I also like this route, perhaps slightly more, depending on how far this
> can be taken:
>
> Disclaimer: I dislike the CSSMetaData and special property subclass a lot
> -- it's a major concession to make properties work with the CSS system that
> brings a lot of boilerplate that is mostly unnecessary.
>
> If you go the route of an annotation, we could offer to do everything via
> the annotation so a regular property can be converted to styleable by
> annotating it. The property would then have to specify the name
> ("-fx-label-padding") and the default value (which can be a string that is
> parseable via the existing CSS parser).
>
> In other words:
>
> @Styleable(name = "-fx-label-padding", defaultValue = "(0, 0, 0, 0)")
> public final ReadOnlyObjectProperty<Insets> labelPaddingProperty() { ... }
> The rest the scanner can figure out (it could even figure out a default
> name). The `isSettable` method is always the same boilerplate (and if not,
> you can use the old way). The required converter can be derived from the
> generic type (Insets -> InsetsConverter), or specified (converter =
> InsetsConverter.class).
>
> To convert a regular property into a styleable does require somewhat more
> magic (the CSS system would have to wrap it, or otherwise track them) but
> that's all hidden. It gets rid of the StyleableProperty uglyness and the
> need for users to create those, and puts the "extra" CSSMetaData
> information a styleable property needs firmly where it belongs (in an
> annotation).
>
> --John
>
> On 06/12/2023 11:37, Nir Lisker wrote:
>
> I thought about the option of reflection, but I opted to propose
> annotations instead. The following is my reasoning.
>
> Firstly, reflection is very magic-y. The author of the class has no
> indication of what side effects happen due to the code they write, the
> output (css handling in this case) comes out of nowhere from their
> perspective. As with other reflection cases, it is a "pull" rather than
> "push" approach - you don't write what should happen, you let someone else
> decide that. For writers of skin/control classes, this means that they need
> to know exactly what constitutes a hook for the reflection mechanism, or
> face surprises. There is no compile time check that tells you whether you
> have declared your styleable property properly or not (without an external
> ad-hoc checker).
> We do this somewhat with properties - any method of the form
> "...property()" gets special treatment, but this is for the docs. I don't
> think we have code that depends on this other than in tests.
>
> Secondly, the proposed mechanism depends on the runtime type, not the
> declared type. As a user, I see no indication in the API whether a property
> is styleable or not. This is also (what I would consider) a problem with
> the current state. When I thought about using reflection to solve this, I
> at least thought to specify the declared type of the property as styleable,
> like StyleableBooleanProperty instead of BooleanProperty (restricting the
> returned type is backwards compatible). A downside of this is that it gives
> access to the methods of StyleableProperty, which are not useful for the
> user, I think, but maybe someone has a use for them.
>
> Thirdly, maybe I want to declare a styleable property not to be used
> automatically. I can't think off the top of my head when I would want to do
> that, but I'm also not a heavy css user. Are we sure that just initializing
> a property with a styleable runtime type should *always* be caught by this
> process?
>
> To compare, annotations have the following benefits:
>
> Firstly, they are declarative, which means no surprises for the class
> author (WYSIWYG). This also allows more flexibility/control over which
> properties get special treatment via an opt-in mechanism.
>
> Secondly, They can also be displayed in the JavaDocs (via @Documented)
> with their assigned values. For example, the padding property of Region can
> be annotated with @Styleable(property="-fx-padding"), informing the user
> both that this value can be set by css, and how to do it. Interestingly,
> the annotation doesn't need to be public API to be displayed, so we are not
> bound by contracts.
>
> In terms of similarities:
>
> In both the reflection and the annotation proposals, the steps are:
> 1. Create styleable properties.
> 2. That's it.
> It's just that step 1 also adds an annotation to the creation of the
> property (which was/is a 2-step process anyway, declaring the property and
> its css metadata).
>
> Annotations also require a processor to read the data from their values
> and target (the field/method). This is a bit of work, but
> Michael's CssMetaDataCache class is basically that - read the data from the
> class (via reflection or annotations) and store it in a map. The logic
> should be the same, just the method to obtain the data is different. Both,
> as a result, have the benefits of handling control/skin combinations (what
> I mentioned in the point "Usable both in controls and in skins (or other
> classes)").
>
> The benefit of co-locating the property and its css metadata in the class
> itself also remains.
>
>
> To summarize, both approaches eliminate all the clutter of writing
> styleable properties (John, will you like to create styleable properties
> now? [1] :) ), both apply the flexibility of caching per class, both allow
> better structuring of the class, but they read the properties differently
> and have a different level of declarativness.
>
> [1]
> https://mail.openjdk.org/pipermail/openjfx-dev/2023-December/044010.html
>
>
> On Tue, Dec 5, 2023 at 11:21 PM Andy Goryachev <andy.goryachev at oracle.com>
> wrote:
>
>> I like the idea.
>>
>>
>>
>> I wonder if it is possible to reduce the amount of boilerplate code? For
>> example, a CssMetaData can have a
>>
>>
>>
>> setGetter(Function<S, StyleableProperty<V>> getter)
>>
>>
>>
>> method which supplies the property reference? This way
>> CssMetaData.isSettable(Node) and CssMetaData.getStyleableProperty(Node) can
>> be implemented in the base class (there are more complicated cases, so
>> perhaps setIsSettable(Predicate<Node>) would also be required).
>>
>>
>>
>> Example:
>>
>>
>>
>> CssMetaData.<ControlExample,Font>of("-fx-font", Font.getDefault(), (n)
>> -> n.font)
>>
>>
>>
>> Just a thought. What do you think?
>>
>>
>>
>> -andy
>>
>>
>>
>>
>>
>> *From: *openjfx-dev <openjfx-dev-retn at openjdk.org> on behalf of Michael
>> Strauß <michaelstrau2 at gmail.com>
>> *Date: *Sunday, December 3, 2023 at 22:02
>> *To: *openjfx-dev <openjfx-dev at openjdk.org>
>> *Subject: *Reflective discovery of styleable properties
>>
>> Following up the discussion around the CssMetaData API, I'd like to
>> chime in with yet another idea. To recap, here's Nir's summary of the
>> current API [0]:
>>
>> "Let's look at what implementation is required from a user who wants
>> to write their own styleable control:
>> 1. Create styleable properties.
>> 2. Create a list of these properties to be passed on.
>> 3. Create a public static method that returns the concatenation of
>> this list with the one of its parent. (This method happens to be
>> poorly documented, as mstr said.)
>> 4. Create a public non-static method that calls the static method in a
>> forced-override pattern because otherwise you will be calling the
>> wrong static method. (This method's docs seem to be just wrong because
>> you don't always want to delegate to Node's list.)"
>>
>>
>> I think this could reasonably be replaced with the following
>> implementation requirements:
>> 1. Create styleable properties.
>> 2. That's it.
>>
>> Let's look at what we're actually trying to do: create a list of
>> CSS-styleable property metadata of a class. But we can easily do that
>> without all of the boilerplate code.
>>
>> When ´Node.getCssMetaData()` is invoked, all public methods of the
>> class are reflectively enumerated, and metadata is retrieved from
>> `Property` and `StyleableProperty` getters. This is a price that's
>> only paid once for any particular class (i.e. not for every instance).
>> The resulting metadata list is cached and reused for all instances of
>> that particular class.
>>
>> As a further optimization, metadata lists are also cached and
>> deduplicated for Control/Skin combinations (currently every Control
>> instance has its own copy of the metadata list).
>>
>> Another benefit of this approach is that the CssMetaData can now be
>> co-located with the property implementation, and not be kept around in
>> other parts of the source code file. Here's how that looks like when a
>> new "myValue" property is added to MyClass:
>>
>> StyleableDoubleProperty myValue =
>> new SimpleStyleableDoubleProperty(this, "myValue") {
>>
>> static final CssMetaData<MyClass, Number> METADATA =
>> new CssMetaData<MyClass, Number>(
>> "-fx-my-value",
>> SizeConverter.getInstance(),
>> USE_COMPUTED_SIZE) {
>> @Override
>> public boolean isSettable(MyClass node) {
>> return !node.myValue.isBound();
>> }
>>
>> @Override
>> public StyleableProperty getStyleableProperty(
>> MyClass node) {
>> return node.myValue;
>> }
>> };
>>
>> @Override
>> public CssMetaData getCssMetaData() {
>> return METADATA;
>> }
>> };
>>
>> public final DoubleProperty myValueProperty() {
>> return myValue;
>> }
>>
>> It is not required to override the `getCssMetaData()` method, nor is
>> it required to redeclare a new static `getClassCssMetaData()` method.
>> It is also not required to manually keep the list of styleable
>> properties in sync with the list of CSS metadata.
>>
>> I've prototyped this concept for the `Node`, `Region` and `Control`
>> classes [1].
>>
>> [0]
>> https://mail.openjdk.org/pipermail/openjfx-dev/2023-December/044046.html
>> [1] https://github.com/openjdk/jfx/pull/1299
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/openjfx-dev/attachments/20231206/32cdd78c/attachment-0001.htm>
More information about the openjfx-dev
mailing list