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

[MT-4431] Refactor - Type system #36

Merged
merged 35 commits into from
Nov 8, 2021
Merged

[MT-4431] Refactor - Type system #36

merged 35 commits into from
Nov 8, 2021

Conversation

e1himself
Copy link
Contributor

@e1himself e1himself commented Nov 1, 2021

  • Invert the type system to define Element as the most generic shape allowing specific implementations to extend it
  • Drop InlineNode type and type-guard in favour of runtime-defined Editor.isInline() check
  • Drop BlockNode type and type-guard in favour of runtime-defined Editor.isBlock() check
  • Drop ElementWithType and its type-guard, as it's the same as the generic Element
  • Drop AdditionalCustomTypes hack and only rely on Slate's stock CustomTypes declaration merging
  • Move all CustomTypes ambient declaration to @prezly/slate-commons (the plan is to keep it there as a temporary solution until we refactor the code to not depend on it)
  • Drop duplicated EmbedElementType definition in favour of EmbedNode defined in @prezly/slate-types
  • Rename LinkCandidateElementType to LinkCandidateNode
  • Rename ImageCandidateElementType to ImageCandidateNode
  • Rename LoaderElementType to LoaderNode
  • Drop duplicated DIVIDER_TYPE constant

image

As it's exactly matching the other place where we override it
Instead of defining it as a union of all possible implementations,
we now declare it as the most generic Element interface (type + children)
and expect all implementations to extend it.

We can also now rely on the new `Element.isElementType()` helper
to reduce duplication in our own type-guards code.
As there's not much need in it now, after we've inverted the types.
This is considered a temporary solution until we figure out
a better way for it. In the longer run, I believe, it's worth
to restructure the code so there is no dependencies or assumptions
to the common Slate editor types structure. But that's not for now.
As it was duplicating `EmbedNode` interface from @prezly/slate-types
@linear
Copy link

linear bot commented Nov 1, 2021

MT-4431 Refactor slate-editor repo types system

While working on MT-4421 we've came to realization that the approach we've took with defining Editor nodes types in the @prezly/slate repository might the root cause of a few problems we've faced:

  • Mixed up type-guards vs data-validation responsibilities of is{…}Node functions. See MT-4421.

  • Rather complex code and types to facilitate the model

  • Inability to extend editor with additional node-types in the application-level repository. This is because generic types like ElementNode, BlockNode and InlineNode are defined as a union of the hard-coded known node types.

    Example:

    Знімок з 2021-10-27 13-53-07.png

  • Inability to provide runtime schema configuration to allow or disallow specific node types for a specific editor instance (this is a logical outcome of limited extensibility — see the point above). It also can cause problems like this: MT-4430.

Our current assumption is that the above can be fixed and improved by:

  • inverting how types are defined
  • moving structural schema definition (and thus validation) to run-time, so every editor instance can define its own supported schema variation

Знімок з 2021-10-27 13-59-08.png

@e1himself e1himself merged commit 55dd462 into master Nov 8, 2021
@e1himself e1himself deleted the refactor/type-system branch November 8, 2021 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant