feat(lift-json)!: Prepare for scala 3 with type extractor#2030
feat(lift-json)!: Prepare for scala 3 with type extractor#2030farmdawgnation merged 5 commits intolift-4.0from
Conversation
Earlier versions of lift-json depended heavily on the old scala Manifest class that has been removed in Scala 3. The plan is to replace with with metaprogramming in Scala 3, but to support that we first have to encapsulate the existing Manifest logic into a separate class. This is the starting point toward doing that.
farmdawgnation
changed the title
|
@dpp Keep me honest, but I think this approach is viable with minimal disruption to the API. :) |
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a TypeExtractor interface to facilitate the migration of lift-json from Scala 2.13 to Scala 3. The PR replaces direct usage of Manifest with the new TypeExtractor abstraction while maintaining backward compatibility through implicit conversions.
Key changes:
- Introduces
TypeExtractortrait as an abstraction over type information extraction - Updates all JSON extraction methods to use
TypeExtractorinstead ofManifest - Provides implicit conversion from
ManifesttoTypeExtractorfor compatibility
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| core/json/src/main/scala/net/liftweb/json/Extraction.scala | Introduces the TypeExtractor trait and updates extraction methods to use it |
| core/json/src/main/scala/net/liftweb/json/JsonAST.scala | Updates extract, extractOpt, and extractOrElse methods to use TypeExtractor |
| core/json/src/main/scala/net/liftweb/json/Serialization.scala | Updates read methods to use TypeExtractor and removes Manifest import |
| core/json/src/main/scala/net/liftweb/json/Formats.scala | Updates field serializer methods to use TypeExtractor |
| core/json/src/main/scala/net/liftweb/json/FieldSerializer.scala | Updates type constraint to use TypeExtractor |
| web/webkit/src/main/scala/net/liftweb/http/LiftSession.scala | Manually applies conversion where implicit conversion failed |
Comments suppressed due to low confidence (1)
core/json/src/main/scala/net/liftweb/json/Extraction.scala:95
- [nitpick] The function name
convertTypeExtractorToClassesis verbose and unclear. Consider renaming toextractClassesorgetAllClassesto better reflect its purpose of extracting all classes from a TypeExtractor hierarchy.
def convertTypeExtractorToClasses(te: TypeExtractor[_]): List[Class[_]] = {
dpp
left a comment
dpp
left a comment
There was a problem hiding this comment.
My Scala 3 is weak, bit this generally looks okay.
But... should we consider dropping Lift JSON for Scala 3/Lift 5 in favor of https://github.com/json4s/json4s
|
@dpp Yeah, agree on the long term strategy being leaning on something else rather than continuing to maintain our own. I'm still concerned about the polymorphism issue. :/ |
|
If you want to preserve compat with the |
3 participants
Provide for a
TypeExtractorinterface to assist with the migration of lift-json to Scala 3. In particular:Manifestdriven implementation in theTypeExtractorManifest=>TypeExtractorthat work in most cases.I took this approach after evaluating how json4s has decided to support Scala 3. By my understanding this was done with the approach of augmenting reflection capabilities. I also realized there's an open bug json4s/json4s#431 (opened by me!) which doesn't seem to have been fixed yet so I'm not eager to try to dig into their reflection implementation to try and fix that and then migrate over.
Going to consider this a breaking change, even though we're going to make the attempt to smooth it over with an implicit conversion.