React Ace and Ace builds

[jmc420]

There's a conflict between React Ace and ace-builds which is described in this github error:

securingsincity/react-ace#1547

React Aces generates a types.d.ts declaration file that looks like this:

interface IRenderer extends Ace.VirtualRenderer {
  placeholderNode?: HTMLDivElement;
  scroller?: HTMLDivElement;
}

This results in a Typescript error:

node_modules/react-ace/lib/types.d.ts:124:11 - error TS2430: Interface 'IRenderer' incorrectly extends interface 'VirtualRenderer'.
  Property 'scroller' is optional in type 'IRenderer' but required in type 'VirtualRenderer'.

124 interface IRenderer extends Ace.VirtualRenderer {

          ~~~~~~~~~

The issue is that scroller must either be optional in both or mandatory in both.

If you clone React Ace and make scroller mandatory in types.ts, you then get a compilation in React Ace:

Type 'Editor' is not assignable to type '{ [index: string]: any; renderer: IRenderer; }'.
    Types of property 'renderer' are incompatible.
      Property 'scroller' is missing in type 'VirtualRenderer' but required in type 'IRenderer'.

206     this.editor = ace.edit(this.refEditor);
        ~~~~~~~~~~~

  src/types.ts:133:3
    133   scroller: HTMLDivElement;
          ~~~~~~~~
    'scroller' is declared here.

Making scroller optional in ace builds' VirtualRenderer interface fixes the problem.

export interface VirtualRenderer extends OptionsProvider, EventEmitter {
    readonly container: HTMLElement;
    readonly scroller?: HTMLElement;

Would it be possible to make this change to line 677 of ace builds' ace.d.ts?

[InspiredGuy]

https://github.com/ajaxorg/ace-builds contains only builds, so I'm moving this issue to the repo with actual ace code https://github.com/ajaxorg/ace

[InspiredGuy]

Hey @jmc420 , this might be caused by the fact that in Ace scroller is of type HTMLElement, and in react-ace it is of type HTMLDivElement, which inherits HTMLElement. So when you try to set an object of HTMLElement type to a property of HTMLDivElement type, this might give you an error.

Suggestion: try to patch react-ace with the change which would make scroller a required HTMLElement instead of optional HTMLDivElement. I'm not aware how react-ace is using the scroller, but HTMLDivElement does not have any differences from HTMLElement according to the spec, except one deprecated property. For this you won't require any changes to Ace itself, since it's react-ace which is using different type.

@nightwing do you see anything that could prevent us from changing the type of VirtualRenderer.scroller and other similar elements to HTMLDivElement? Seems like they all are created as divs.

[jmc420]

Thanks for the reply.

The issue is about scroller being optional on line 133 of types.ts in react-ace:

scroller?: HTMLDivElement;

https://github.com/securingsincity/react-ace/blob/main/src/types.ts

On line 678 of ace.d.ts scroller is mandatory:

readonly scroller: HTMLElement;

https://github.com/ajaxorg/ace-builds/blob/master/ace.d.ts

I checked out react-ace and tried to change scroller so that it is mandatory rather than optional on line 133 of types.ts:

https://github.com/securingsincity/react-ace/blob/main/src/types.ts

This results in compilation errors in React ace:

src/ace.tsx:206:5 - error TS2322: Type 'Editor' is not assignable to type 'IAceEditor'.
  Type 'Editor' is not assignable to type '{ [index: string]: any; renderer: IRenderer; }'.
    Types of property 'renderer' are incompatible.
      Property 'scroller' is missing in type 'VirtualRenderer' but required in type 'IRenderer'.

206     this.editor = ace.edit(this.refEditor);

if I edit ace.d.ts (within node_modules) and make scroller optional, the error goes away.

[InspiredGuy]

I checked out react-ace and tried to change scroller so that it is mandatory rather than optional on line 133 of types.ts:
https://github.com/securingsincity/react-ace/blob/main/src/types.ts
This results in compilation errors in React ace:

@jmc420 And this is most probably caused by react-ace using a different type for scroller (HTMLDivElement) than ace (HTMLElement). You cannot assign object of type HTMLElement to a field of type HTMLDivElement, and I suspect this is giving you the compilation error. Ace.VirtualRenderer.scroller is of type HTMLElement, and IRenderer.scroller is of type HTMLDivElement, and those are not the same. Have you tried the suggestion from my previous comment, to change the type of scroller in react-ace to scroller: HTMLElement?

[jmc420]

Have you tried the suggestion from my previous comment, to change the type of scroller in react-ace to scroller: HTMLElement?

That does not make any difference. It still complains that scroller is required in VirtualRenderer.

[InspiredGuy]

Since VirtualRenderer in ace already contains scroller (which was not the case before the version which triggered this issue in react-ace), maybe removing it from IRenderer completely would help? it could look like this, based on the code in the issue description.

interface IRenderer extends Ace.VirtualRenderer {
  placeholderNode?: HTMLDivElement;
}

Technically, setting scroller to optional in ace would be incorrect since VirtualRenderer creates it in constructor body and it's not deleted in VirtualRenderer code, so it's always there. Making it optional could break types for people, relying on it being non-optional.

[jmc420]

maybe removing it from IRenderer completely would help? it could look like this, based on the code in the issue description.

This results in compilation errors:

src/ace.tsx:558:23 - error TS2339: Property 'scroller' does not exist on type 'VirtualRenderer & IRenderer'.

558       editor.renderer.scroller.removeChild(editor.renderer.placeholderNode);
                          ~~~~~~~~

src/ace.tsx:567:23 - error TS2339: Property 'scroller' does not exist on type 'VirtualRenderer & IRenderer'.

567       editor.renderer.scroller.appendChild(node);
                          ~~~~~~~~

[InspiredGuy]

In this case I suggest to create an issue in react-ace, or comment on existing one (if any), since this is not an issue with the type of scroller in ace, but rather a compatibility issue (react-ace did not update it's types after ace v1.5.2 was released on May 30, 2022, which included the types update). Closing this as wontfix.