Add macro requirements #2

2025-09-03T15:39:53Z

BobdaFett commented

2025-09-03 15:39:53 +00:00

(Migrated from github.com)

Creates two new traits: AsReactive and ReactiveCapture. They are responsible for converting between Reactive and non-reactive types.

These traits are primarily derived using the Reactive proc macro, however I found that in its current state, this macro cannot (easily) procedurally create a conversion between custom and built-in types. I put as much description in the doc comment for the Reactive macro.

I recommend that any custom functionality, such as conversion between reactive and non-reactive field types within structs, should be implemented manually, possibly with the assistance of a second macro.

Creates two new traits: `AsReactive` and `ReactiveCapture`. They are responsible for converting between `Reactive` and non-reactive types. These traits are primarily derived using the `Reactive` proc macro, however I found that in its current state, this macro cannot (easily) procedurally create a conversion between custom and built-in types. I put as much description in the doc comment for the `Reactive` macro. I recommend that any custom functionality, such as conversion between reactive and non-reactive field types within structs, should be implemented manually, possibly with the assistance of a second macro.

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed

2025-09-03 15:43:42 +00:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull Request Overview

This PR introduces a trait-based system for converting between reactive and non-reactive types in a Rust application using Leptos. The changes establish a standardized interface for creating reactive versions of structs and capturing their values back to non-reactive forms.

Adds AsReactive and ReactiveCapture traits that define conversion patterns between reactive and non-reactive types
Updates the Reactive proc macro to generate trait implementations instead of standalone methods
Migrates existing code to use the new trait-based approach

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
traits/src/reactive.rs	Defines the core `AsReactive` and `ReactiveCapture` traits with comprehensive documentation
traits/src/lib.rs	Creates the traits module and re-exports the traits
traits/Cargo.toml	Defines the new traits crate with Leptos dependency
src/pages/home_page.rs	Updates code to use trait methods instead of direct constructor calls
src/components/multi_entry.rs	Adds `Reactive` derive and updates formatting
macros/src/lib.rs	Refactors macro to generate trait implementations and improves documentation
macros/Cargo.toml	Adds dependency on the traits crate
Cargo.toml	Updates edition to 2024 and adds traits dependency

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

## Pull Request Overview This PR introduces a trait-based system for converting between reactive and non-reactive types in a Rust application using Leptos. The changes establish a standardized interface for creating reactive versions of structs and capturing their values back to non-reactive forms. - Adds `AsReactive` and `ReactiveCapture` traits that define conversion patterns between reactive and non-reactive types - Updates the `Reactive` proc macro to generate trait implementations instead of standalone methods - Migrates existing code to use the new trait-based approach ### Reviewed Changes Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | traits/src/reactive.rs | Defines the core `AsReactive` and `ReactiveCapture` traits with comprehensive documentation | | traits/src/lib.rs | Creates the traits module and re-exports the traits | | traits/Cargo.toml | Defines the new traits crate with Leptos dependency | | src/pages/home_page.rs | Updates code to use trait methods instead of direct constructor calls | | src/components/multi_entry.rs | Adds `Reactive` derive and updates formatting | | macros/src/lib.rs | Refactors macro to generate trait implementations and improves documentation | | macros/Cargo.toml | Adds dependency on the traits crate | | Cargo.toml | Updates edition to 2024 and adds traits dependency | </details> --- <sub>**Tip:** Customize your code reviews with copilot-instructions.md. <a href="/R15-Software-Dev/scholarships-rs/new/main/.github?filename=copilot-instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Create the file</a> or <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">learn how to get started</a>.</sub>

macros/src/lib.rs

						
				@@ -1,7 +1,72 @@

				use proc_macro::TokenStream;

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-09-03 15:43:42 +00:00

This line attempts to call as_reactive() on an RwSignal<CustomType>, but RwSignal doesn't implement AsReactive. The correct approach would be to get the value first: let custom_reactive: CustomTypeReactive = user_reactive.custom.get().as_reactive();

/// let custom_reactive: CustomTypeReactive = user_reactive.custom.get().as_reactive();

This line attempts to call `as_reactive()` on an `RwSignal<CustomType>`, but `RwSignal` doesn't implement `AsReactive`. The correct approach would be to get the value first: `let custom_reactive: CustomTypeReactive = user_reactive.custom.get().as_reactive();` ```suggestion /// let custom_reactive: CustomTypeReactive = user_reactive.custom.get().as_reactive(); ```

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-09-03 15:43:42 +00:00

There's a typo in 'impmlemented'. It should be 'implemented'.

/// macro is probably not the way to go - as of now, this functionality should be implemented manually.

There's a typo in 'impmlemented'. It should be 'implemented'. ```suggestion /// macro is probably not the way to go - as of now, this functionality should be implemented manually. ```

traits/src/reactive.rs

						
				@@ -0,0 +78,4 @@

				    type CaptureType: AsReactive;

				    /// Captures the current state of the reactive struct into a non-reactive struct.

				    fn capture(&self) -> Self::CaptureType;

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-09-03 15:43:42 +00:00

The capture method takes &self but the documentation example shows it being called with self. Consider changing this to fn capture(self) -> Self::CaptureType to match the example and consume the reactive struct, or update the documentation to show &self usage.

    fn capture(self) -> Self::CaptureType;

The `capture` method takes `&self` but the documentation example shows it being called with `self`. Consider changing this to `fn capture(self) -> Self::CaptureType` to match the example and consume the reactive struct, or update the documentation to show `&self` usage. ```suggestion fn capture(self) -> Self::CaptureType; ```

BobdaFett (Migrated from github.com) reviewed

2025-09-03 15:47:50 +00:00

traits/src/reactive.rs

						
				@@ -0,0 +78,4 @@

				    type CaptureType: AsReactive;

				    /// Captures the current state of the reactive struct into a non-reactive struct.

				    fn capture(&self) -> Self::CaptureType;

BobdaFett (Migrated from github.com) commented

2025-09-03 15:47:49 +00:00

We do not want to consume the reactive struct while capturing it - that would defeat the purpose of building the reactive struct in the first place. Using &self makes sense in this situation.

In this case, this means that the documentation is incorrect. I will change this with the next commit.

We do not want to consume the reactive struct while capturing it - that would defeat the purpose of building the reactive struct in the first place. Using `&self` makes sense in this situation. In this case, this means that the documentation is incorrect. I will change this with the next commit.

BobdaFett (Migrated from github.com) reviewed

2025-09-03 15:48:53 +00:00

macros/src/lib.rs

						
				@@ -1,7 +1,72 @@

				use proc_macro::TokenStream;

BobdaFett (Migrated from github.com) commented

2025-09-03 15:48:53 +00:00

This is correct - will change with next commit.

BobdaFett (Migrated from github.com) reviewed

2025-09-03 15:49:30 +00:00

macros/src/lib.rs

						
				@@ -1,7 +1,72 @@

				use proc_macro::TokenStream;

BobdaFett (Migrated from github.com) commented

2025-09-03 15:49:30 +00:00

Figures.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: r15-software-dev/scholarships-rs#2

				`@@ -1,7 +1,72 @@`
				`use proc_macro::TokenStream;`

				`@@ -1,7 +1,72 @@`
				`use proc_macro::TokenStream;`

				`@@ -1,7 +1,72 @@`
				`use proc_macro::TokenStream;`