Feature/7232 multiple wrap components

[DayS]

This PR fixes the issue #7232 : Adding multiple plugins with wrapComponents on the same core component seems broken

A new option pluginsBehavior has been added to keep backward compatibility for existing plugins.
When this flag is set to chain, the system.combinePlugins() method chain the plugins targeting the same core component, by providing the system to inner systemExtend call.

Motivation and Context

I'm currently working on a set of plugins and stumble upon this issue as multiple of them needed to wrap info component.
Also, I think the project lacks some unique plug-points (like the JumpToPath which is used in multiple context).

Checklist

My PR contains...

• No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
• Dependency changes (any modification to dependencies in package.json)
• Bug fixes (non-breaking change which fixes an issue)
• Improvements (misc. changes to existing features)
• Features (non-breaking change which adds functionality)

My changes...

• are breaking changes to a public API (config options, System API, major UI change, etc).
• are breaking changes to a private API (Redux, component props, utility functions, etc.).
• are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
• are not breaking changes.

Documentation

• My changes do not require a change to the project documentation.
• My changes require a change to the project documentation.
• If yes to above: I have updated the documentation accordingly.

Automated tests

• My changes can not or do not need to be tested.
• My changes can and should be tested by unit and/or integration tests.
• If yes to above: I have added tests to cover my changes.
• If yes to above: I have taken care to cover edge cases in my tests.
• All new and existing tests passed.

[tim-lai]

@DayS Thanks for the PR, it looks good in general, and thanks for considering possibly breaking changes. A couple of feedback points:

• instead of pluginsBehavior, let's use pluginsOptions: {}. So then we would be checking for pluginsOptions.pluginLoadType === 'chain'
• add a default pluginsBehavior object to the defaults in src/core/index.js, as well as pluginsOptions.pluginLoadType: 'legacy'
• add documentation for this new config option, and recommend that if one does NOT want the chain behavior, one should start adding legacy to their code now.

By adding pluginsOptions, we enable possible future options, while defining the default pluginsOptions.pluginLoadType: legacy gives indication that we are likely to change the default in the future.

If you have any additional questions or thoughts, please feel free to share. Thanks again!

[DayS]

Hi, thanks for the review.

The pluginsOptions object seems like a better idea, I'll change that. Do we still need the plugin prefix on pluginLoadType ? It seems a little redundant.

And just to be sure :

• add a default pluginsBehavior

You mean pluginsOptions, right ?

[tim-lai]

The pluginsOptions object seems like a better idea, I'll change that. Do we still need the plugin prefix on pluginLoadType ? It seems a little redundant.

I tend to favor verbosity. 😄

You mean pluginsOptions, right ?

correct. typo.

[DayS]

The PR has been updates with the changes. Let me know if it seems clear enough for the doc changes

[tim-lai]

@DayS Whoops on my part, can you you revert commit 0fe6fbf "Apply suggestions from code review". Thanks.

[tim-lai]

@DayS PR merged! Thanks for the collaboration and contribution!

[DayS]

Awesome !

You're welcome. I'm thrilling to see the next release and use this :)