Skip to content

feat(type): optimize parameter type inference for the execute function of the slash menu #524

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

Closed
wants to merge 6 commits into from

Conversation

zcf0508
Copy link

@zcf0508 zcf0508 commented Jan 18, 2024

This PR optimizes parameter type inference for the execute function of the slash menu.

  1. now the return of getDefaultSlashMenuItems function be able to infer types correctly
assertType<BaseSlashMenuItem<
  typeof defaultBlockSchema,
  any,
  any
>[]>(getDefaultSlashMenuItems(defaultBlockSchema))
  
assertType<BaseSlashMenuItem<
  BlockSchemaFromSpecs<{
    test: typeof testBlock
  }>,
  any,
  any
>[]>(getDefaultSlashMenuItems({
  test: testBlock['config'],
}))
  1. now the parameter of slash menu execute function be able to infer types correctly
const editor = BlockNoteEditor.create({
  blockSpecs: {
    ...defaultBlockSpecs,
    test: testBlock,
  },
  slashMenuItems: [
    ...getDefaultSlashMenuItems(),
    {
      name: "table",
      execute: (_editor) => {
	 // ↓ here
        assertType<BlockNoteEditor<DefaultBlockSchema & BlockSchemaFromSpecs<{
          test: typeof testBlock
        }>>>(_editor)

      },
    },
  ],
});

Close #191 (comment)

Copy link

vercel bot commented Jan 18, 2024

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

Name Status Preview Updated (UTC)
blocknote ✅ Ready (Inspect) Visit Preview Jan 24, 2024 4:33am

Copy link

vercel bot commented Jan 18, 2024

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

A member of the Team first needs to authorize it.

@YousefED
Copy link
Collaborator

Hi @zcf0508 !

Very interesting, thanks for providing this PR and especially for including new tests.

I'm not sure the taken approach is necessary. I didn't know yet about NoInfer, that seems interesting for sure.

However, I think the main issue you're trying to solve can also be fixed by removing the default value for schema here (which means removing = defaultBlockSchema as unknown as BSchema):

export const getDefaultSlashMenuItems = <
  BSchema extends BlockSchema,
  I extends InlineContentSchema,
  S extends StyleSchema
>(
  schema: BSchema = defaultBlockSchema as unknown as BSchema
)

I think removing that fixes most of your tests and keeps the code simple.

However, regardless of this, we're still running into an issue in Editor.tsx (e.g.: InsertAlert). This is also why your build fails. I'm not sure there's a good way of fixing this (even with NoInfer), except when we change the API maybe (to a generic-form of getInsertAlert for example). What do you think?

@zcf0508
Copy link
Author

zcf0508 commented Jan 22, 2024

I'm sorry that I forgot to run build script to ensure my changes are compatible. The code should be able to build success now.

My idea is to let generics obtain as much information as possible. So I don't care about the defaultBlockSchema, it has no impact on the inference of generics.

@YousefED
Copy link
Collaborator

@zcf0508 the build error is still there

@zcf0508
Copy link
Author

zcf0508 commented Jan 24, 2024

Yeah, I see the build error.

I think we need that when users define menu using defineReactSlashMenuItem, the options can correctly infer the block schema of the parameter editor of options.execute.

So we need to refactor the definition of useBlockNote. We can add a wrapper type for BlockNoteEditorOption like ReactBlockNoteEditorOption, and the slashMenuItems of ReactBlockNoteEditorOption will be rewritten to Array<ReactSlashMenuItem>.

type ReactBlockNoteEditorOptions<
  BSpecs extends BlockSpecs,
  ISpecs extends InlineContentSpecs,
  SSpecs extends StyleSpecs
> = Omit<BlockNoteEditorOptions<BSpecs, ISpecs, SSpecs>, 'slashMenuItems'> & {
  slashMenuItems: Array<
    ReactSlashMenuItem<
      BlockSchemaFromSpecs<NoInfer<BSpecs>>,
      any,
      any
    >
  >
};

And then we need change the options type of initEditor and useBlockNote.

const initEditor = <
  BSpecs extends BlockSpecs,
  ISpecs extends InlineContentSpecs,
  SSpecs extends StyleSpecs
>(
  options: Partial<ReactBlockNoteEditorOptions<BSpecs, ISpecs, SSpecs>>
) => ...

export const useBlockNote = <
  BSpecs extends BlockSpecs = typeof defaultBlockSpecs,
  ISpecs extends InlineContentSpecs = typeof defaultInlineContentSpecs,
  SSpecs extends StyleSpecs = typeof defaultStyleSpecs
>(
  options: Partial<ReactBlockNoteEditorOptions<BSpecs, ISpecs, SSpecs>> = {},
  deps: DependencyList = []
) =>  ...

Now, we can add slashMenuItem in useBlockNote directly.

const editor = useBlockNote({
  domAttributes: {
    editor: { class: styles.editor, "data-test": "editor" },
  },
  blockSpecs,
  slashMenuItems: [
  {
    name: "Insert Alert",
    execute: (editor) => {   //   the `BSchema` of `editor` will be `BlockSchemaFromSpecs<typeof blockSpecs>`
      editor.insertBlocks(
        [
          {
            type: "alert",
          },
        ],
        editor.getTextCursorPosition().block,
            "after"
          );
        },
        aliases: [
          "alert",
          "notification",
          "emphasize",
          "warning",
          "error",
          "info",
          "success",
        ],
        group: "Media",
        icon: <RiAlertFill />,
        hint: "Insert an alert block to emphasize text",
    },
  ],
});

I think this is a good way for users to define a menu item. But this will prevent users from defining menu item out of useBlockNote.

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.

2 participants