Skip to content

feat!: STJ Experiment#1370

Open
JR-Morgan wants to merge 26 commits into
devfrom
jedd/cnx-3322-json-improvements-placeholder
Open

feat!: STJ Experiment#1370
JR-Morgan wants to merge 26 commits into
devfrom
jedd/cnx-3322-json-improvements-placeholder

Conversation

@JR-Morgan
Copy link
Copy Markdown
Member

@JR-Morgan JR-Morgan commented Apr 20, 2026

see specklesystems/speckle-sharp-sdk#468 for the SDK changes.

This PR updates this repo to use the new System.Text.Json (STJ) powered SDK.

See https://www.notion.so/speckle/Introducing-System-Text-Json-to-packfile-based-serializer-SendPipleine-357b78fc7aa6805bb472ca09b980b7fd for context.


PR Round 2

The first version of this PR bumped the minimum version of the SDK to 8.0.0. This was bad for several reasons.
(compatibility with older Revit plugins, and potentially dangerous situations in Rhino .NET7/4.8)

This this new version of the PR is far more safe. Our min compatible STJ version is now 5.0.0; (same as what v2 connectors used). No longer conflicts with any other plugins.
Downsides are pretty minimal, it does limit our SDK to 5.0.0 API, but this is workable.

For .NET 8/10 targets

  • No change dependency from main, we're good.

For .NET4.8 (including Rhino8):

  • Plugins that that are compatible with any version ≤8.0.6 STJ will not be broken by speckle
  • Plugins that load STJ 5.0.0 (or greater) guaranteed to not break Speckle.
  • tl;dr very little opportunity for dll conflicts

@linear
Copy link
Copy Markdown

linear Bot commented Apr 20, 2026

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

❌ Patch coverage is 0% with 49 lines in your changes missing coverage. Please review.
✅ Project coverage is 31.41%. Comparing base (7e4c13d) to head (057f743).
⚠️ Report is 9 commits behind head on dev.

Files with missing lines Patch % Lines
...le.Connectors.Common/AssemblyCompatibilityCheck.cs 0.00% 44 Missing ⚠️
...ckle.Connectors.Common/Operations/SendOperation.cs 0.00% 4 Missing ⚠️
Sdk/Speckle.Connectors.Common/SpeckleLogger.cs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1370      +/-   ##
==========================================
- Coverage   31.93%   31.41%   -0.52%     
==========================================
  Files         115      116       +1     
  Lines        3285     3339      +54     
  Branches      358      365       +7     
==========================================
  Hits         1049     1049              
- Misses       2197     2251      +54     
  Partials       39       39              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JR-Morgan
Copy link
Copy Markdown
Member Author

We should also test Grasshopper, and Rhino under .net7 (rather than .net8)

@JR-Morgan
Copy link
Copy Markdown
Member Author

JR-Morgan commented May 1, 2026

Pulling my hair out a bit, I thought that dropping our min verison to 5.0.0 would be the end of any conflicts. But it looks like the .NET Standard 2.0 STJ dll is not compatible with .NET 4.6.2 STJ dll.

This is very strange, so needs more investigation.
as it is right now, we still have a conflict with Data Exchange, because we require .NET Standard2.0 dll, and it requires .NET 4.6.2. Normally these would be compatible, but for what ever reason, they are not 😢.
This is probably a bug in STJ that was never fixed in 5.x

Adding a .net 4.6.2 target to our SDK would probably solve this, but that would need a discussion.

A better solution might be to use some reflection to work around this. I'll try Activator.CreateInstance to see if that's more forgiving.

Fixed by downgrading System.Memory from 4.5.5 -> 4.5.3

@JR-Morgan JR-Morgan marked this pull request as ready for review May 8, 2026 13:55
@JR-Morgan JR-Morgan requested a review from oguzhankoral as a code owner May 8, 2026 13:55
@JR-Morgan
Copy link
Copy Markdown
Member Author

Current status:
we've released this as a beta (see https://speckle.community/t/connector-performance-improvements-beta/22277)

no reports of problems currently, but we haven't seen that many downloads yet. so I want to give this longer to simmer.

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.

2 participants