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

Modify Text interface to be compatible with "universal syntax tree" (unist) #4378

Open
JonasKruckenberg opened this issue Jul 15, 2021 · 12 comments
Labels

Comments

@JonasKruckenberg
Copy link
Contributor

Problem
The unified ecosystem provides a vast amount of utility functions for tree traversal, transformation and validation. It also supports easy parsing of html and markdown out of the box. We (and I can image others too) would love to leverage those tools in combination with slate. However both trees are not compatible: Slate expects leaf nodes of the following shape:

interface Text {
   text: string
}

while leaf (literal) nodes defined in unist have the following shape:

interface Literal {
   type: "text",
   value: string
}

Solution
As you can see the only incompatibility is the name of the value key text vs value (And just renaming that across slate does the trick)

Context
In my fork I have a version of slate that is fully compatible with unist-style trees. I was just wondering if support for this is something you're interested in and if I should therefore put the work into a PR. It would most definitely result in a breaking change so I can understand if you don't.

@JonasKruckenberg
Copy link
Contributor Author

@horacioh FYI

@cameracker cameracker changed the title Unified compatibility Modify Text interface to be compatible with "universal syntax tree" (unist) Aug 9, 2021
@raitucarp
Copy link

I upvote this. Unified gives a good ecosystem. Hopefully, in the future, Slate will join the ecosystem.

@dylans
Copy link
Collaborator

dylans commented Aug 12, 2021

I personally am a fan of this change (not sure how others on the core slate team feel), though it's a breaking change and it would have significant impact on users that persist slate documents.

I can see a couple of paths towards getting this to work, assuming this is a change that the Slate team wants to support:

  • Make the key configurable (with a default of text, and some future plan to change that default to value)
  • Support text or value as the key and have the code base treat them interchangeably
  • Change from text to value and introduce a breaking change at some point in the future (maybe 1.0)

@cameronbraid
Copy link

I've come across this issue as I have written some code that converts the slate syntax tree into a uinst compatible syntax tree (and back again) to be able to use uinst transform, visit etc in my apps.

Is this proposal being seriously considered by the slate team ?

@cameronbraid
Copy link

I should also note that its not only the Text interface that needs to be changed, as a unist Node requires a type: string field as well

@dylans
Copy link
Collaborator

dylans commented Apr 8, 2022

I think if we followed my proposed approach we could make it work, along with your note. Interested in raising a PR?

@horacioh
Copy link
Contributor

horacioh commented Apr 8, 2022

@JonasKruckenberg wdyt?

@JonasKruckenberg
Copy link
Contributor Author

That's super exciting! I had some issues in the past especially with making the types generic over the key, but that's something we can figure out for sure

@horacioh
Copy link
Contributor

almost a year after! 🤷🏼‍♂️

@dylans about point one and make the text key variable, how do you think that can be done? here are the ways @JonasKruckenberg and I discussed:

  1. add an extra property to the options of each function: This could work but is a massive change, not sure if the effort will justify the result?
  2. add a global context with the value: adding things to the global object does not seem like a good practice for a library like Slate, not sure if this was an option

would love your input on this one, we are maintaining a fork right now because of this and we would love to contribute on this to the library so we can all benefit and we can ditch our fork too! 😅

thanks in advance!

@JonasKruckenberg
Copy link
Contributor Author

JonasKruckenberg commented Jul 14, 2022

Well 2. is really a non-option, and the major downside of 1. is horrible DX bc you'd need to pass the option to literally every slate function if you want a custom key:

Transforms.insertNode(editor, node, { textKey: 'value' })`

Text.isText(value, { textKey: 'value' })

Node.string(node, { textKey: 'value' })

Really no great options :/

@dylans
Copy link
Collaborator

dylans commented Jul 16, 2022

Well 2. is really a non-option, and the major downside of 1. is horrible DX bc you'd need to pass the option to literally every slate function if you want a custom key:

Transforms.insertNode(editor, node, { textKey: 'value' })`

Text.isText(value, { textKey: 'value' })

Node.string(node, { textKey: 'value' })

Really no great options :/

Something like:

// BaseEditor interface
// Overrideable core actions.

getTextKey: () => string

//If you want to override the default, you provide something like:

getTextKey: () => return 'value'

// But then each time the code did something like:

const text = node.text.slice(offset)

// we would need to do:

node[getTextKey()].slice(offset)

// we don't currently as far as I know have any global editor options, 
// but if the above was a performance bottleneck, we could add textKey 
// on the editor itself and then do

node[editor.textKey].slice(offset)

// I haven't profiled function calls vs. property lookups in a while so I don't have a strong opinion here

@NourIM
Copy link

NourIM commented Oct 4, 2023

Hi @cameronbraid! Can you share a sandbox sample of converting slate into uinst, please? It's going to be very beneficial for me as I'm trying to do the same!
Our ecosystem is already dealing with the Slate doc now and there is no point to get back to let slate editor exporting unist, so I need the both presentation!

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

No branches or pull requests

6 participants