Skip to content

Conversation

@jevansaks
Copy link
Member

@jevansaks jevansaks commented May 22, 2024

Description

This change moves AdaptiveCards.Templating to be built on System.Text.Json and the flavor of AdaptiveExpressions that also uses System.Text.Json. It was necessary to move off of Newtonsoft to be able to support AOT.

I have preserved the old API, but added AOT compatibility attributes and added AOT compatible versions instead. The AdaptiveCards.Templating library fundamentally works against json strings, and the APIs which took object would immediately serialize that object to a json string. The main changes are:

  • All APIs taking object from the application now take string, making it the caller's responsibility to serialize to json.
  • object-based APIs remain for compatibility but are flagged as RequiresDynamicCode and RequiresUnreferencedCode.
  • I changed the unit tests to compile as AOT compatible and changed them where needed.

How Verified

Ran all existing unit tests and they passed.

Note: I had to make one minor change to an expected test result because System.Text.Json does not round-trip literal double quotes in a string, they come back escaped not as \" but as \u0022. It seems that Newtonsoft had different behavior around this.

@jevansaks jevansaks marked this pull request as ready for review June 25, 2024 19:13
@jevansaks
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 8919 in repo microsoft/AdaptiveCards

@anna-dingler
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jwoo-msft
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jwoo-msft
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jwoo-msft jwoo-msft merged commit 59dad36 into microsoft:main Jun 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants