Skip to content

Conversation

@xecrets
Copy link
Contributor

@xecrets xecrets commented Oct 25, 2025

What kind of change does this PR introduce?
This is a small update to the Readme, based on the discussion System.Text.Json-attribute support #318 concerning how to use ReactiveUI.SourceGenerators with other source generators, specifically System.Text.Json.

What is the current behavior?
There is no mention of this little caveat, possibly causing confusion and increased support work load for the maintainers.

What is the new behavior?
A brief section explaining the background of Roslyn source generator limitations concerning inter-dependencies, and short explanation of how to work around this by placing the dependency source generator in a separate assembly.

What might this PR break?
It's just text in the Readme, so it's unlikely to break anything.

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features) - N/A
  • Docs have been added / updated (for bug fixes / features) - Check.

Other information: N/A

@glennawatson glennawatson requested a review from Copilot October 25, 2025 15:58
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds documentation to the README explaining how to use ReactiveUI.SourceGenerators alongside other source generators, specifically addressing the System.Text.Json source generator compatibility issue discussed in #318.

  • Explains Roslyn source generator limitations regarding inter-dependencies and execution order
  • Provides a workaround by recommending separate assemblies for dependent source generators
  • Clarifies that attributes emitted by one generator aren't visible to others in the same compilation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

so adding `[JsonPropertyName]`/`[JsonInclude]` from `ReactiveUI.SourceGenerators` won’t cause the `System.Text.Json`
source generator to pick them up in that project. That’s by design of the generator pipeline (no inter-generator dependencies / ordering).

So `System.Text.Json` needs special care. In the case that you want to Json-serialize `[Reactive]`
Copy link

Copilot AI Oct 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'Json-serialize' to 'JSON-serialize' for consistency with standard JSON abbreviation capitalization.

Suggested change
So `System.Text.Json` needs special care. In the case that you want to Json-serialize `[Reactive]`
So `System.Text.Json` needs special care. In the case that you want to JSON-serialize `[Reactive]`

Copilot uses AI. Check for mistakes.
@glennawatson glennawatson enabled auto-merge (squash) October 25, 2025 15:59
@glennawatson
Copy link
Contributor

Cool thanks. I'm sure this will help people out. The interdependent source generator thing has tripped me up in the past so it'll help people out

@glennawatson glennawatson merged commit 0ff4332 into reactiveui:main Oct 25, 2025
4 checks passed
@xecrets xecrets deleted the feature/sg-json-readme branch October 25, 2025 16:05
@github-actions
Copy link

github-actions bot commented Nov 9, 2025

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants