Merry Christmas and sorry for late reply - haven't been coding as much as am currently doing a course in South Korea :) I've been thinking about this for a while, but hadn't been able to come to a firm conclusion.

The biggest advantages I see for using the create* pattern are that it's reasonably consistent with the previous mxGraph, and that the generic type of the plugin will be known by the returning method. The disadvantages I can see are that if we wanted to maintain reasonable backwards compatibility with previous methods in the Graph class, many of the create* handlers like createEdge would need to be called createEdgeInstance or something similar, creating inconsistencies.

The biggest advantages I see for reusing the plugins argument are that it's clear by looking in one place what class handlers will be overridden. The biggest disadvantage I could think of was that using getPlugin(...) might need to be coerced using <...>getPlugin(..) or getPlugin(...) as ... in order to have the advantages of TypeScript's strict typing and autocompletion.

However, by a quick stack overflow search I found this set of answers: https://stackoverflow.com/questions/54165536/typescript-function-return-type-based-on-input-parameter. Based on this I think it may be possible to mitigate against the typing issues, while keeping consistency with the previous plugins argument and minimising the need to explain method naming exceptions for backwards compatibility.

For these reasons, I'm currently thinking merging the GraphInstantiators options with the plugins argument, and having some specific logic for those handlers which should be created on-demand outside the Graph constructor (such as EdgeHandler) might be best.

What are your thoughts @tbouffard, do you think reusing the plugins argument would be best, or do you think it would be best to move things to create* methods as used in the previous mxGraph? If we wanted to move GraphInstantiators properties to the create* methods, I think it may be worth considering moving all the plugins to create* methods for consistency so we don't have multiple means of doing the same thing, as I think that could make things more complex to understand for end users of the library.

Wow, how's life in Seoul? Let me know if you need any help :)

Adding type support to the plugins is definitely doable. I can work on it once we decide which way to go.

Hi everybody and happy 2023 🎉. I wish you all the best for this coming year.
Sorry for the late answer, the end of 2022 was very busy 🎅🏿

I agree 100% with all the arguments for moving to the GraphInstantiators/plugins way.
My concern in my initial comment was mainly about consistency 😸
About the mxGraph migration with the removal of the create* methods, as far we document it, this should be manageable for anyone (as it has a limited scope). The new implementation will bring some improvements that deserve the change IMHO.

The mxGraph codebase was not fully consistent about its extension points and all of them had drawbacks

• Global states (CellRenderer for instance) impacting all instances of Graph living in an application
• top-level prototype impacting all sub-classes which can be the desired behavior or can introduce side-effects silently
• some extensions were missing (the Graph.createEdgeHandler mentioned in a previous comment in this discussion)
• create* methods: to override a Collaborator class and use it, you must override the implementation of all classes using it (by overriding the related create*methods) instead of just using the new Collaborator with existing class

mxGraph also didn't separate responsibilities with the create* methods pattern. The class/object knows how to instantiate its collaborators, it manages all the dependencies graph. This introduces the usual drawbacks in this case: coupled code, tendency to God object (as we see with the mxGraph class), difficulties to unit test the code in isolation, ....

About the implementation of the new pattern

• I suggest we do not implement it directly in feat: restore remaining non-"Editor" examples to Storybook #150.
  • This PR fixes examples, the fix can be done without the proposed changes
  • The changes we want to do introduce breaking changes and we will have to update some examples to make them match the new way of doing. Mixing refactoring and fixes in an single PR is too much IMHO
• for the "plugins" types support, let's keep in mind that we may want to support extension i.e. let user introduce their own plugins.
  • this came to me because we currently have extensibility issues in CellStateStyle (for instance, the type of the shape property is not opened to extension anymore - I will fix this soon).
  • the chosen implementation may prevent to use types with custom plugins. For instance, in case we use a closed list of plugins strings to detect the plugin type
  • we can also defer the possibility to let user introduce their own plugins, but adding it later may require to introduce new breaking changes
  • I have currently no opinion about the need for plugins extensibility. As we are still in alpha, there is no rush to take such decision. It is probably wiser to start without extensibility to be able to move forward. Then, get feedback and see if there is a demand for such a thing.

@junsikshim thanks :D amazing how fast-paced and how much emphasis there is on technical knowledge+things, have picked up a bunch of books on NLP for Korean. Hoping can use the techniques in them to analyse sentences+use maxGraph to visualise them for language learning purposes :)

@tbouffard hny!! 😸 Agree totally about separating into different commits. I think it may be best to commit the plugin changes first, merge into the examples branch, then merge the refactored examples later on. I think if we need to add plugins extensibility we may also be able to create a generic "any" fallback type, although I suspect it may be best to keep things to strict types and cast as needed to reduce the possibility of bugs