SimpleJSON being used instead of System.Text.Json

[alexandrehtrb]

I was just taking a look at PR #108 and honestly, it looks kind of dangerous.
SimpleJSON last code update was 2 years ago, while STJ is actively maintained by Microsoft.

Instead of changing the entire code only to support Unity, we could use conditional compilation, keeping the original code as it was and using SimpleJSON only for Unity.

On the csproj:

<!-- if Unity then SimpleJSON -->
<!-- else use System.Text.Json -->
<ItemGroup Condition=" '$(TargetFramework)' == 'net462' ">
    <Reference Include="System.Web" />
</ItemGroup>

If it involves C# code, preprocessor directives can be used.

[danipen]

Could you please define dangerous?

[alexandrehtrb]

Dangerous for two reasons:

1. It is maintained by an unknown developer. I am not accusing him of anything; but code from a well-known and respected maintainer (Microsoft) is more reliable and trustworthy.

2. This change modified a lot of code and this can lead to regression bugs.

[danipen]

Thanks for the clarification.

I understand the concerns about maintenance and the size of the change. That said, I’m not planning to introduce conditional compilation or maintain two different code paths/builds.

Keeping both System.Text.Json and SimpleJSON would add extra complexity to the project, increase maintenance cost, and make the codebase harder to reason about over time.

The current implementation is well covered by unit tests (all grammars and themes), and those tests give us a high level of confidence that the SimpleJSON-based solution works correctly. From that point on, the risk of regressions is already mitigated by testing.

Regarding trust and security: I agree it’s a factor to consider, but given the scope and usage here, and the test coverage we have, it’s an acceptable tradeoff to keep the project simpler and more maintainable.

So for now, this change is intentional and I don’t plan to revert it. If someone strongly prefers the previous approach, they can continue using an older version or maintain a fork with their own changes.

I appreciate you raising the concerns and taking the time to review the PR.

[udlose]

@danipen out of curiosity, can I ask the reason to move away from STJ?

[danipen]

The main reason is Unity integration.

TextMateSharp is starting to be included directly in Unity projects. When using System.Text.Json, we need to pull in a fairly large set of additional dependencies. In Unity, adding this kind of common dependency is risky, as it increases the chance of version conflicts and collisions with other packages already present in a project.

By switching away from System.Text.Json, we can avoid those extra dependencies entirely. This significantly simplifies the Unity package (and also reduces its footprint).

[udlose]

Gotcha - thanks for the clarification.