Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Edittor Settings in XML while exporting Entity as Source Control #43

Open
amacierzynski opened this issue Jul 19, 2022 · 5 comments
Open

Comments

@amacierzynski
Copy link

amacierzynski commented Jul 19, 2022

Hi!
Great job about this editor! During early tests, I found out that info about the view state is being kept in the Entity object itself. That is causing an issue with keeping that metadata in XML while exporting to source control entities.

image

Normally I would not expect to keep that metadata persistent. I see the value of keeping it during development, but not permanently.

I've developed a quick alternative with sessionStorage and it works perfectly fine. Metadata is stored there temporarily when Tab is open. Each service will have its own key-value pair:

image

Let me know if you are interested in having it in your codebase. Can do a fork and PR later on.

@stefan-lacatus
Copy link
Collaborator

This would actually be a great improvement.

The current implementation was a quick PoC to prove that this functionality can be implemented, but it was never improved, so it faces a few issues:

  1. It pollutes the entity itself, and is visible in the XML exports, causing problems when using source control.
  2. The "viewstate" is only saved when the entity itself is saved. This works nicely when the user actively works on a service, but is very annoying when the user just wants to view the code (readonly), without clicking save at the end.
  3. It lacks preserving the undo/redo stack (Get/Restore Monaco editor Undo&Redo stack microsoft/monaco-editor#686), and it wasn't looked into because saving it in XML would have been too big.

Your implementation could work around issues 1. and 2., while opening the door to fixing 3. as well. Here's some points I think would improve it even more:

  1. Use localStorage instead of sessionStorage (see here). When working in ThingWorx I almost always have at least 2-3 Composer tabs open. The editors in them should keep the same "viewState". Additionally, the "viewState" should be preserved between restarts/session expiration
  2. Improve naming of the keys: Something like: ${EntityType}_${EntityName}_ViewState
  3. Optional, would resolve point 2. of the above, but can also be done at a later date: save the viewState whenever the editor closes, even if it was readonly.

I would gladly accept a PR with this functionality. Thanks a lot in the interest of using this extension.

@amacierzynski
Copy link
Author

@stefan-lacatus
My implementation was also PoC. Glad that you are interested in my proposal.
I will work on that considering your comments as well.

About Undo/Redo stack I have a few doubts. How would you resolve the problem of working on the same code with more than 1 person at the same time? Saving this stack in XML would be for sure bad, but I am not sure if localStorage would be any better. As far as I remember it has some strict limits in terms of storage size in general and single string length. Probably depends on the browser also.

@amacierzynski
Copy link
Author

amacierzynski commented Jul 20, 2022

Improve naming of the keys: Something like: ${EntityType}_${EntityName}_ViewState

This is also quite not ok IMHO. I would assume that Entity_Name is fine, but we are missing a service name for sure here.
What u think about standard EntityName_ServiceName_ViewState?

@stefan-lacatus
Copy link
Collaborator

stefan-lacatus commented Jul 20, 2022

Improve naming of the keys: Something like: ${EntityType}_${EntityName}_ViewState

This is also quite not ok IMHO. I would assume that Entity_Name is fine, but we are missing a service name for sure here. What u think about standard EntityName_ServiceName_ViewState?

you are 100% correct on that. Completely forgot about the service name

What about ${EntityType}_${EntityName}_${MethodName}_ViewState? This will for sure avoid any collisions.

P.S. I call it method name because subscriptions also must work, and MethodName fits both "serviceName" and "subscriptionName"

@stefan-lacatus
Copy link
Collaborator

We can merge this in multiple steps, with multiple PRs:

  1. just replace the current storage with localStorage.
  2. make it save viewState when the editor closes, not just when the entity closes.
  3. look into the undo/redo stack. Here we definitely need to do some work
    • monaco does not offer any official APIs for that
    • as the localStorage limit seems to be ~5MB per origin. Some rough napkin math is that this will allow us to preserve a stack 10 deep, where each diff between checkpoints is ~8000 chars long (this is extreme, would mean that the difference between each editor change you would undo is a full change to ~200 lines), for 50 services, fitting us into ~4MB. So some smartness needs to be added around what is persisted.
    • to also address your point: " How would you resolve the problem of working on the same code with more than 1 person at the same time?": well i don't think there's anything to be addressed. The localStorage is per browser, so each developer would end up with their own undo stack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants