v8 - Typescript build errors on `heading.options`

greggmoritz commented 1 week ago

Reproduction Steps

Using CKEditor React component from @ckeditor/ckeditor5-react@7.0.0

Editor configuration includes the following heading options and is building as expected:

heading: {
options: [
  { model: 'paragraph', title: 'Paragraph' },
  {
    model: 'heading1',
    view: {
      name: 'h1',
      classes: 'text-primary-500',
    },
    title: 'Heading 1',
  },
  {
    model: 'heading2',
    view: {
      name: 'h2',
      classes: 'text-primary-500',
    },
    title: 'Heading 2',
  },
  {
    model: 'heading3',
    view: {
      name: 'h3',
      classes: 'text-primary-500',
    },
    title: 'Heading 3',
  },
  {
    model: 'heading4',
    view: {
      name: 'h4',
      classes: 'text-primary-500',
    },
    title: 'Heading 4',
  },
  {
    model: 'heading5',
    view: 'h5',
    title: 'Heading 5',
  },
  {
    model: 'heading6',
    view: 'h6',
    title: 'Heading 6',
  },
],
},

Upgrade CKEditor5 to v42 per instructions on https://ckeditor.com/docs/ckeditor5/latest/updating/nim-migration/predefined-builds.html. Everything still building as expected.
Upgrade ckeditor-react to v8.0.0

Expected

Continue to build successfully with no code changes

Actual

Typescript build error:

../core/components/richtexteditor/RichTextEditor.tsx:23:7 - error TS2322: Type '{ plugins: (typeof BlockQuote | typeof Bold | typeof Essentials | typeof GeneralHtmlSupport | typeof Heading | typeof ImageBlock | typeof ImageInline | ... 19 more ... | typeof Alignment)[]; ... 7 more ...; ckbox: { ...; }; }' is not assignable to type 'EditorConfig'.
  The types of 'heading.options' are incompatible between these types.
    Type '({ model: string; title: string; view?: undefined; } | { model: string; view: { name: string; classes: string; }; title: string; } | { model: string; view: string; title: string; })[]' is not assignable to type 'HeadingOption[]'.
      Type '{ model: string; title: string; view?: undefined; } | { model: string; view: { name: string; classes: string; }; title: string; } | { model: string; view: string; title: string; }' is not assignable to type 'HeadingOption'.
        Type '{ model: string; title: string; view?: undefined; }' is not assignable to type 'HeadingOption'.
          Property 'class' is missing in type '{ model: string; title: string; view?: undefined; }' but required in type 'HeadingCustomElementOption'.

23       config={editorConfig}
         ~~~~~~

  ../.yarn/cache/@ckeditor-ckeditor5-heading-npm-42.0.0-5b197d7b12-bd99aaf4a3.zip/node_modules/@ckeditor/ckeditor5-heading/src/headingconfig.d.ts:114:5
    114     class: string;
            ~~~~~
    'class' is declared here.
  ../.yarn/__virtual__/@ckeditor-ckeditor5-react-virtual-8a2f7b8144/0/cache/@ckeditor-ckeditor5-react-npm-8.0.0-215f637ae2-48125621f8.zip/node_modules/@ckeditor/ckeditor5-react/dist/ckeditor.d.ts:115:5
    115     config?: EditorConfig;
            ~~~~~~
    The expected type comes from property 'config' which is declared here on type 'IntrinsicAttributes & IntrinsicClassAttributes<CKEditor<ClassicEditor>> & Pick<Readonly<Props<ClassicEditor>>, keyof Props<...>> & Pick<...> & Pick<...>'

Found 1 error in ../core/components/richtexteditor/RichTextEditor.tsx:23

This looks like the type definition has changed (I did not see this documented), so I added a class: '' key/value to the "paragraph" model config and rerun the build. Now I see

../core/components/richtexteditor/RichTextEditor.tsx:23:7 - error TS2322: Type '{ plugins: (typeof BlockQuote | typeof Bold | typeof Essentials | typeof GeneralHtmlSupport | typeof Heading | typeof ImageBlock | typeof ImageInline | ... 19 more ... | typeof Alignment)[]; ... 7 more ...; ckbox: { ...; }; }' is not assignable to type 'EditorConfig'.
  The types of 'heading.options' are incompatible between these types.
    Type '({ model: string; title: string; class: string; view?: undefined; } | { model: string; view: { name: string; classes: string; }; title: string; class?: undefined; } | { model: string; view: string; title: string; class?: undefined; })[]' is not assignable to type 'HeadingOption[]'.
      Type '{ model: string; title: string; class: string; view?: undefined; } | { model: string; view: { name: string; classes: string; }; title: string; class?: undefined; } | { model: string; view: string; title: string; class?: undefined; }' is not assignable to type 'HeadingOption'.
        Type '{ model: string; title: string; class: string; view?: undefined; }' is not assignable to type 'HeadingOption'.
          Type '{ model: string; title: string; class: string; view?: undefined; }' is not assignable to type 'HeadingCustomElementOption'.
            Types of property 'model' are incompatible.
              Type 'string' is not assignable to type '`heading${string}`'.

23       config={editorConfig}
         ~~~~~~

  ../.yarn/__virtual__/@ckeditor-ckeditor5-react-virtual-8a2f7b8144/0/cache/@ckeditor-ckeditor5-react-npm-8.0.0-215f637ae2-48125621f8.zip/node_modules/@ckeditor/ckeditor5-react/dist/ckeditor.d.ts:115:5
    115     config?: EditorConfig;
            ~~~~~~
    The expected type comes from property 'config' which is declared here on type 'IntrinsicAttributes & IntrinsicClassAttributes<CKEditor<ClassicEditor>> & Pick<Readonly<Props<ClassicEditor>>, keyof Props<...>> & Pick<...> & Pick<...>'

Found 1 error in ../core/components/richtexteditor/RichTextEditor.tsx:23

All of the heading options model values are headingN and the paragraph option should not be matching the HeadingCustomElementOption type. This is blocking me from upgrading to the latest version of ckeditor5-react

Environment Details

Yarn 4.30 using Yarn Plug'n'Play (pnp)
Vite 5.3.0
React 18.3.1
Browser: Chrome (NA)
OS: macOs Sonoma 14.5
First affected CKEditor-React version: v8

Witoso commented 1 week ago

I think I created a similar sample with Vite + React + TS (no yarn pnp), and the issue is not observed. Apart from the IDE errors (they appear in standard Stackblitz setup), the npx tsc is clear.

https://stackblitz.com/edit/vitejs-vite-cvkmdt?file=src%2FApp.tsx

Could you share a reproducible sample that we could use to debug this faster?

filipsobol commented 1 week ago

The class property is required on all heading options, and has been for at least 1.5 years when we migrated to TypeScript (source and API docs). I don't know why TypeScript didn't catch this earlier in your project.

greggmoritz commented 1 week ago

I think I created a similar sample with Vite + React + TS (no yarn pnp), and the issue is not observed. Apart from the IDE errors (they appear in standard Stackblitz setup), the npx tsc is clear.

https://stackblitz.com/edit/vitejs-vite-cvkmdt?file=src%2FApp.tsx

Could you share a reproducible sample that we could use to debug this faster?

Thanks for your response @Witoso. I'll work on a repro sample.

ckeditor / ckeditor5-react