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

Fix: propSchema data-types and TS autocomplete. #264

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

leweyse
Copy link

@leweyse leweyse commented Jun 30, 2023

My use-case for BlockNote expects to take advantage as much as possible of Custom Blocks and their customization. This PR enables the use of data-types of props for the propSchema definition, and it also fixes the TS autocomplete for props.

The library allows us to update our Blocks' props by using updateBlock and the only reason to not use Objects, Arrays or Booleans as default values in our propSchema is the type definition, however it is possible to update the props with any type of values not breaking the app. I look forward to keep my app type-safe by using more data-types than strings for the props of my new Custom Blocks.

Also, after propSchema is defined, the type inference for props is lost here:

editor: BlockNoteEditor<BSchema & { [k in Type]: BlockSpec<Type, PSchema> }>

Once autocompletion was enabled, I needed to update the defaultProps for defaultBlocks, as well as the test values for formatConversions.test.ts.

@vercel
Copy link

vercel bot commented Jun 30, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
blocknote ✅ Ready (Inspect) Visit Preview Jul 10, 2023 8:21am
blocknote-website ✅ Ready (Inspect) Visit Preview Jul 10, 2023 8:21am

@matthewlipski
Copy link
Collaborator

Hi, thanks for contributing to BlockNote! Custom blocks are still very much a WIP, and we'll be adding new functionality to them over time, such as the ability to have props that have different types, not only strings. The reason why prop types are limited to strings at the moment is because we always render them to HTML attributes, which must be strings.

I think instead of making the types of all custom props unknown, we want to probably use a generic/type argument instead. I'm also a bit wary of rendering objects as HTML attributes, since this can make the HTML pretty unreadable, so I reckon if a prop is not a primitive type, it should not be rendered as an attribute. Ideally we would let users define custom parsing and rendering of props but I think this should be ok to start with. Let me know if you'd like to keep working on this, otherwise I'd probably work on it myself at a later time.

@leweyse
Copy link
Author

leweyse commented Jul 4, 2023

Hi @matthewlipski! Thanks for the information. Your point makes sense, I didn't consider the props to be rendered as attributes. I'd like to continue working on this.

Also, I have a question. Is there a place/branch to follow the progress/discussion about Custom Blocks? I'd like to follow the relevant new changes closely and, if possible, contribute to them.

@matthewlipski
Copy link
Collaborator

Hmm, unfortunately right now we don't have any central place for tracking the progress of issues/features - since the BlockNote team consists of only myself and @YousefED, our communication and planning is pretty ad-hoc😅. Since we're getting more community contributions now it might be a good idea to have a public issue board so people can stay updated.

For now we have a roadmap, though it's pretty vague and not something we update often.

@leweyse leweyse marked this pull request as draft July 5, 2023 09:17
@vercel
Copy link

vercel bot commented Aug 4, 2023

@leweyse is attempting to deploy a commit to the TypeCell Team on Vercel.

A member of the Team first needs to authorize it.

@leweyse leweyse marked this pull request as draft August 4, 2023 14:52
@leweyse leweyse marked this pull request as ready for review August 7, 2023 10:02
@leweyse
Copy link
Author

leweyse commented Aug 7, 2023

Hi guys!

@matthewlipski I've reviewed this issue and finally implemented a way to represent block attributes with complex data types in strings.

@YousefED from your last comment, I take the approach to serialize and deserialize JSON for complex data attributes. This comes with some extra steps to, for example, update the props. To not remove any other properties (enhanced with TS), we need to add the previous state to the new object (example). In regard PM and y-pm, I haven't tested these changes yet.

I updated the example for testing purposes, but let me know if you prefer to remove those changes.

I hope you can review this PR soon. Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants