feat(generator-plugin-transfertypes): add generics & importing from the correct module #3318
Conversation
…metadata to transfer types
…ng instead of an array
| public Signal<String> getStringSignal() { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
I think we should actually prevent this case from happening, though, it is possible in java.
There was a problem hiding this comment.
What do you mean? It's just a test file, isn't it? All we need is a method signature here.
There was a problem hiding this comment.
There shouldn't be possibility of returning Signal<String>, but only its concrete implementations such as ListSignal, ValueSignal, NumberSignal for now. Not sure, where is it the correct place to put a check on this.
|
| @@ -0,0 +1,3 @@ | |||
| package com.vaadin.hilla.signals; | |||
|
|
|||
| public record Signal<T>() {} | |||
There was a problem hiding this comment.
This file may not be needed, since there shouldn't be a possibility to have Signal as the return type in any endpoint methods.
There was a problem hiding this comment.
Not actually sure why should there be two sets of placeholder classes for the signals, one with annotations on top of them, and one without any annotations. Why wouldn't putting the newly introduced annotation on top of the actual implementation of the ListSignal, ValueSignal, NumberSignal, and others work?
| if (namedSpecifier.isBlank() | ||
| && defaultSpecifier.isBlank()) { | ||
| throw new IllegalArgumentException(String | ||
| .format("@FromImport annotation for class %s must have at least one named specifier or a default specifier", |
There was a problem hiding this comment.
I assume this is typo:
| .format("@FromImport annotation for class %s must have at least one named specifier or a default specifier", | |
| .format("@FromModule annotation for class %s must have at least one named specifier or a default specifier", |
| @@ -0,0 +1,3 @@ | |||
| package com.vaadin.hilla.signals; | |||
|
|
|||
| public record Signal<T>() {} | |||
There was a problem hiding this comment.
Not actually sure why should there be two sets of placeholder classes for the signals, one with annotations on top of them, and one without any annotations. Why wouldn't putting the newly introduced annotation on top of the actual implementation of the ListSignal, ValueSignal, NumberSignal, and others work?
This pull request includes several changes across multiple files to enhance functionality and improve the codebase. The most important changes involve adding new annotations, refactoring methods, and updating dependencies. Below is a summary of the key changes:
Enhancements and New Features:
@FromModuleto specify module information for transfer types inpackages/java/runtime-plugin-transfertypes/src/main/java/com/vaadin/hilla/transfertypes/annotations/FromModule.java.Signalclasses and corresponding test cases to handle different signal types inpackages/java/parser-jvm-plugin-transfertypes/src/test/java/com/vaadin/hilla/signals/. [1] [2] [3] [4]Code Refactoring:
scanmethod inPlugininterface to provide a default implementation inpackages/java/parser-jvm-core/src/main/java/com/vaadin/hilla/parser/core/Plugin.java.TransferTypesPluginto handle new signal classes and added logic to replace signal classes with local signal records inpackages/java/parser-jvm-plugin-transfertypes/src/main/java/com/vaadin/hilla/parser/plugins/transfertypes/TransferTypesPlugin.java. [1] [2]Dependency and Import Management:
TransferTypesPluginand other related files.module-info.javato export the newannotationspackage inpackages/java/runtime-plugin-transfertypes/src/main/java/module-info.java.Testing and Validation:
packages/java/parser-jvm-plugin-transfertypes/src/test/java/com/vaadin/hilla/parser/plugins/transfertypes/signals/SignalTest.java.These changes collectively enhance the functionality and maintainability of the codebase by introducing new features, refactoring existing methods, and ensuring proper dependency management.