feat: add 0:n override

[utnapischtim]

• this change enables to add multiple components to one override id

[ThiefMaster]

Can you please share an example on how this would be used?

[ThiefMaster]

Also, wouldn't it be cleaner to override the component with a custom component (this could potentially be a React.Fragment in case no props are needed) that contains your list of new components as children?

[utnapischtim]

I am working on a cataloging feature for the repository of tu graz based on InvenioRDM.

this commit tries it out:
tu-graz-library/invenio-catalogue-marc21@36ac95b

it is mostly here where i add the Overridable container

here i add the custom components to the store. this should at the end be moved to a separate package. it is now tested within the catalogue-marc21 to make the tests easier. at the end i will move that to invenio-workflows-tugraz and invenio-alma.

here i added the context provider. it is the wrapper round the whole deposit page.

[utnapischtim]

Also, wouldn't it be cleaner to override the component with a custom component (this could potentially be a React.Fragment in case no props are needed) that contains your list of new components as children?

no, because i would like to have the possibility to add e.g. UploadCSV from the example to invenio-workflows-tugraz and ImportFromAlma to the package invenio-alma

so the idea is that different packages can add features to the ManageRecord component

[ThiefMaster]

Suggested change

	const elements = overrides.map((overriden, i) =>
	React.createElement(overriden, {...childProps, ...restProps, key: `${id}-${i}`})
	const elements = overrides.map((overridden, i) =>
	React.createElement(overridden, {...childProps, ...restProps, key: `${id}-${i}`})

[ThiefMaster]

OK, sounds reasonable. I'll let one of the Invenio guys have a look at the change as well though before testing it myself (just to ensure it does not break anything in Indico, even though the change looks fully backwards compatible) and merging it.

[utnapischtim]

i moved one component to another package to check if all is still working as expected.

diff for invenio-workflows-tugraz: https://github.com/tu-graz-library/invenio-workflows-tugraz/pull/112/files . it adds the component here to the overrideStore

and the tricky part was to add the invenio-workflows-tugraz-publisher.js to be called. i solved it by using a configuration variable here which is used here and then in the jinja html here

[ntarocco]

Also, wouldn't it be cleaner to override the component with a custom component (this could potentially be a React.Fragment in case no props are needed) that contains your list of new components as children?

@utnapischtim I tend to agree with @ThiefMaster, and I would be more in favor of keeping this library simple as it is.

Question (I didn't check the code deeply): can we instead introduce here an extension point, and you can implement extra APIs like .append in your instance, or in InvenioRDM?

@ThiefMaster ideas? Do you see a use case for this in Indico installations?

[utnapischtim]

we could add a separate Expandable instead of adding it to Overridable

like: #25

but i think if we want to have this solution it would be better to create a separate react package like react-expandable and maybe it should be found a better name.
adding a new component with another name goes against the package name. therefore i thought adding it to Override would be the better idea.

[utnapischtim]

Also, wouldn't it be cleaner to override the component with a custom component (this could potentially be a React.Fragment in case no props are needed) that contains your list of new components as children?

@utnapischtim I tend to agree with @ThiefMaster, and I would be more in favor of keeping this library simple as it is.

Question (I didn't check the code deeply): can we instead introduce here an extension point, and you can implement extra APIs like .append in your instance, or in InvenioRDM?

i am unsure what you mean, but in my interpretation it would introduce more "complicated" code to the react-overridable package as what i added.

[utnapischtim]

@ThiefMaster
i would like to ask what the current status of this PR is?

[ThiefMaster]

Should this fail if there's already a single-component override? Or automatically convert the existing entry to an array containing that one?

Right now add followed by append silently discards the initial element.

[utnapischtim]

I would say that is debatable.

After thinking it through i would say, append should silently convert the existing one element to an array with the element in it.

BUT: what do we do if afterwards a add is called?

after thinking that another time through (i already feel like i am a reasoning model) i would use another property for append

  append = (id, Component) => {
    if (!Array.isArray(this.components_multiple[id])) { // TODO: naming
      this.components_multiple[id] = [];
    }
    this.components_multiple[id].push(Component);
  };

with following change:

  get = id => {
    let out = [];
    if (id in this.components_multiple) {
       out = this.components_multiple[id];
    }

    if (id in this.components) {
      out.push(this.components[id])
    }

    if (out.length > 1) {
       return out;
    } else {
        return out[0];
    }
  };

with this approach we would enable the option to add another item to a container which is already replaced.

[utnapischtim]

i close this. i have implemented the feature now in my package