Update injector for re-mounted components#138
Conversation
|
@Ni55aN I've run the tests locally and I've updated the previous post. I've got green-light for angular >= 15. Sadly I couldn't test for angular < 15 because it didn't build (the error is unrelated to the changes, I've tested with a vanilla |
idk probably I had some problems with WSL, but yeah v14 seems to have build problems not related to my changes. |
|
thanks! I tested the changes for the case when a node with the same identifier is mounted in a new editor. Since the editor has newly initialized services in the DI, the node now refers to the new services instead of the old ones |
|
🎉 This PR is included in version 2.3.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
Description
Hi all, this is my first contribution here, please let me know if I missed something obvious. I wanted to open an issue but since the fix was quite straightforward I jumped ahead and opened this PR.
I noticed that with the current implementation if you override the default IDs created by rete with your own the renderer would not correctly update the injector context for custom elements. As stated here, the limitation was quite known already.
This PR partially addresses the limitation but at least allows the next-created elements to correctly get a new injector with the correct instances.
Related Issues
None.
Checklist
Additional Notes
I'm wondering if it is really correct to have as the identifier of the CustomElement the identifier of the node. Wouldn't be better to use the component class name? or something that does not change for each data payload contained in the node? In my understanding, in the current solution if your retejs instance contains N nodes then we are defining N different CustomElement even if we would probably need way lass (one of each CustomElement type defined by the developer).