Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Immutable Data Structure #84

Open
wakaztahir opened this issue Jul 30, 2023 · 9 comments
Open

Support Immutable Data Structure #84

wakaztahir opened this issue Jul 30, 2023 · 9 comments
Assignees
Labels
enhancement New feature or request

Comments

@wakaztahir
Copy link
Contributor

wakaztahir commented Jul 30, 2023

The initial importing and exporting using HTML or Markdown is good but its not the one that fits every requirement

Problem

Suppose I have a backend, I can store the output of this editor as HTML or Markdown , Now that goes to my database, I have a website which is built using React which needs to display this rich text in another editor which is built using React or Javascript, If I store it using HTML, well I must be able to parse and display the HTML exactly like this editor does otherwise differences will be introduced. If I store markdown, I will have to make sacrifices on features that markdown doesn't support, An immutable data structure makes it very easy

We make very clean, immutable data classes storing only primitive types like strings, representing the rich text in editor

Solution

A immutable data structure model must be created

Since data classes would be immutable, The structure will be clear so user knows all the types of models our editor has suppose ImmutableParagraph, ImmutableRichText(val isBold, ...)

The data structure would only contain properties that the editor supports and to methods to convert to the mutable data structure that is editable when editor is running

for example

internal data class ImmutableRichTextParagraph(
    val items : List<ImmutableRichItem>
){
    fun toRichTextParagraph() : RichTextParagraph {
        val para = RichTextParagraph()
        para.addSpans(items.map { it.toRichTextSpan() })
        return para
    }
}

In Kotlin, usually immutable classes don't start with immutable word and instead mutable classes have mutable in their names like List and MutableList, since mutable classes are already internal, This change could be made any time when the data structure is mature enough

Pros

  • Standardizes Everything in Immutable Data Classes that Rich Text Editor supports
  • This immutable data structure can be taken out and separated into core library
  • More export and import formats can be supported by just creating libraries that would convert to this immutable data structure
  • Clean code with better organization and structure

Cons

  • Maintain compatibility with previous versions of data structure (if supporting JSON serialization)
  • More code

Thoughts on Exposing Core Data Structure

The data structure must be separated out to another library core, The editor must make implementation dependency on core, so Users cannot access the data structure unless they explicitly specify a implementation dependency on the core library

This would solve

  • No need to have internal on each data class, making it unable to use with a solid use case
  • Hide data structure from users, so library API is not polluted and allow to work on the model until its stable

Where when the user gives HTML , The HTML parser parses the html to this immutable data structure, When markdown is given the parser converts to this immutable data structure

Supporting JSON Serialization

If users were to store the immutable model as json in their databases, We must maintain compatibility backwards

  • Renaming a property, we must use @JsonNames("previous_property_name") annotation from kotlinx.serialization on the new property
  • Removing a property, Either we must make sure user deserializes his json using the option ignoreUnknownKeys in kotlinx.serialization library, or we need to make it nullable and set its default value to null and make the property deprecated
  • Adding a property, No requirement, since if we added a property in new version of the library, user can only generate json with that property using the new version of the library, previous version of the library just cannot deserialize it

Let me know if you need any help, I can contribute

@wakaztahir wakaztahir changed the title Support Immutable Data Structure Support Immutable Data Structure or Introduce a builder API Jul 31, 2023
@wakaztahir
Copy link
Contributor Author

wakaztahir commented Jul 31, 2023

A builder API is required to be able to convert from existing data structure to Rich Text Editor

Currently classes have constructors that are ambiguous , requiring children and text and other values that must be left blank or given, This doesn't make sense as whether one should supply text or children
If a builder API is introduced , User's could convert to and from their own data structure to a model that rich text requires
Builder API can make it simpler with multiple functions with same names but different types

buildParagraph {
   addText("my text")
   addText("my text", makeBold = true)
}
buildParagraph("complete text of the paragraph")
buildParagraph("complete text of the paragraph",Alignment.Left)

Builder API makes JSON deserialization user's responsibility, They can traverse on their data and build rich text state using builder pattern

Warning
However, JSON Serialization still requires traversing the RichTextState and there's no public facing API for that , for JSON serialization and deserialization, supporting immutable data classes is still a better choice.

I will contribute a PR on this but in a distant future as soon as I am free

@MohamedRejeb
Copy link
Owner

I will take a look at this as soon as I get time.

@wakaztahir wakaztahir changed the title Support Immutable Data Structure or Introduce a builder API Support Immutable Data Structure Aug 3, 2023
@MohamedRejeb
Copy link
Owner

That sounds interesting but there's only one problem with this approach, which is that react app for example doesn't know anything about this structure and they need to implement a lot of logic to transform that JSON to HTML and to transform the HTML back to JSON, it's a lot of additional work and in this process the HTML could be changed as well so problem is not solved.

@wakaztahir
Copy link
Contributor Author

wakaztahir commented Aug 5, 2023

No, We don't need to care about how other editors display our data structure

A lot of rich text editors for web (good ones) have their own data structure, They don't require users to type out HTML to display (of course), When we type out JSON, The rich text editor converts JSON to HTML however they like, and display it for users to edit and then allow exporting JSON

If I wanted to use another rich text editor on my website, I would just need to convert whatever is stored in my database to the data structure or JSON that their rich text editor supports

I could even go further and invent my own data structure and store that in my database and whenever I want to display it in a rich text editor, I just convert it to the data structure that that rich text editor supports, this is what I should do because both rich text editors could change their data structure any time

https://editorjs.io/

Three Choices

  • STORE HTML OF EDITOR -> it can change, plus I would need to convert it to HTML that other rich text editor supports
  • STORE MARKDOWN OF EDITOR -> yes both rich text editor support markdown, but markdown has missing features, markdown is not the ideal format
  • STORE JSON OF EDITOR -> I'd always be sure that rich text editor supports everything stored in JSON and works with it, even if the HTML in the editor changes, Editor doesn't need to support JSON serialization but an immutable data structure, that I can convert to from my own JSON

@MohamedRejeb
Copy link
Owner

Ah ok I get your point, that would be really cool. You need to consider not using data classes. We can't provide data classes as public APIs in libraries because they don't support binary compatibility so the classes needs to be just normal class with hashCode and toString implementations.

@wakaztahir
Copy link
Contributor Author

Good to have

  • All the properties of classes should be immutable, meaning val not var
  • Classes shouldn't have multiple properties for the same thing like ->
    • for example children & text, Instead class should have a property children, and a secondary constructure that takes text as parameter & converts text to children and calls primary constructor

@MohamedRejeb
Copy link
Owner

MohamedRejeb commented Aug 5, 2023

  • I will create a new branch called core and the work about this feature is going to be merged there until its finished.
  • Create new richeditor-core module
  • The classes could be similar to the existing mutable one.
  • For the naming I suggest renaming the existing classes to MutableClassNameand name the new ones ClassName.
  • We can move as much code as we can to the new
  • Decouple some classes from compose (ex: removing TextRange...)

@MohamedRejeb MohamedRejeb added the enhancement New feature or request label Aug 5, 2023
@wakaztahir
Copy link
Contributor Author

wakaztahir commented Aug 5, 2023

I will take some time, before I start, I will do this...
Another question, why do we need to maintain binary compatibility, isn't API compatibility enough, every time users use new version of library, they are going to recompile the app with the new version too, then why ?

@MohamedRejeb
Copy link
Owner

Take a look at this article by Jake Wharton: https://jakewharton.com/public-api-challenges-in-kotlin/
Also we are not gaining any advantages by using data class here so can use just a simple class

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants