Improve color widget picker and types

[sneridagh]

No description provided.

[netlify]

✅ Deploy Preview for plone-components canceled.

Name	Link
🔨 Latest commit	5161f5b
🔍 Latest deploy log	https://app.netlify.com/sites/plone-components/deploys/660fd0054dfa6500089c1b4d

[netlify]

✅ Deploy Preview for volto canceled.

Name	Link
🔨 Latest commit	5161f5b
🔍 Latest deploy log	https://app.netlify.com/sites/volto/deploys/660fd0054db61b0008defd7c

[davisagli]

@sneridagh This doesn't seem right to me. These are not interfaces with a fixed set of keys. They are mappings which can take an arbitrary number of string: React.ComponentType entries, to register new widget types. Isn't there some way we can express that in a type without needing to list all these?

[sneridagh]

The beauty of a TS interface is that you can extend them as in:

declare module '@plone/types' {
  export interface WidgetsConfigByWidget {
    BackgroundColorWidget: React.ComponentType<any>;
  }
}

anywhere, eg. in your add-on.

https://github.com/kitconcept/kitconcept.com/blob/main/frontend/src/addons/volto-kitconcept_com/src/index.ts#L50-L53

[davisagli]

@sneridagh Yeah but it seems ridiculous if I have to update an interface just to declare that there is a new type of widget that I'd like to register, and then also actually register it.

I think these types should be declared as Record<string, React.ComponentType<any> -- https://dmitripavlutin.com/typescript-record/

[sneridagh]

@davisagli You are right, in this one makes sense, because all have the same shape (They are all Record<string, React.ComponentType), in others is not that easy, and TS is not that smart when all the records do not have the same shape, or if you have to start writing complex unions.

I'd like also to standardize and make people know and use the extending interfaces pattern, so my reasoning path went in that way too. But it's true that as much as we can prevent people from writing code, we should.

Let me try it out how well it works and I will make a PR.

[sneridagh]

@davisagli Trying it out, I remembered why :) So it's all about autocompletion too. With a generic record you cannot have both. Also this is not allowed:

export interface WidgetsConfig = Record<WidgetsConfigKeys, React.ComponentType<any>>

And you can't do:

export interface WidgetsConfig {
  default: React.ComponentType;
  id: WidgetsConfigById;
  widget: WidgetsConfigByWidget;
  vocabulary: WidgetsConfigByVocabulary;
  factory: WidgetsConfigByFactory;
  choices: React.ComponentType;
  type: WidgetsConfigByType;
  views: WidgetsConfigViews;
  [k in string]: Record<string, React.ComponentType<any>>;
}

Since Interfaces can't declare both static and dynamic keys:

https://ts-error-translator.vercel.app/?error=IIAgtghgDlCmAmIAuBPO4IpAOwPZJHlgGMAbCAJ1hCgtzgqQEtYBnEXC8WJAC13isAdEA

Since interfaces don't work like this, so you have to fall back to types, then they are not extendable.
So we can only have both extensibility and autocompletion using this pattern.

Let's talk about this on Monday!