figma-plugin icon indicating copy to clipboard operation
figma-plugin copied to clipboard

DRAFT: Feat/add new provider for remote generic versioned storage

Open SorsOps opened this issue 2 years ago • 11 comments

This is with reference to https://github.com/six7/figma-tokens/discussions/1112 . It exposes the same interface as the existing JSONBin storage provider, however it abstracts away the specifics on the JSONBin endpoint such as its hardcoded url , fixed headers ,etc and allows those to be set in the storage provider settings in the case that someone would like to create their own endpoint that is read/write. I understand the remote URL storage provider is ready only at the moment.

@six7 One of my concerns is how to manage array data for the storage provider, your current create and edit storageitem modals use a passed in onChange event which expects to be interacting with a raw input element, and then sets the property on the data object directly, which works for simple key value associations, but not particularly well when array or nested object operations occur. As such I've tested a prototype using a set operation notation which would be the least disruptive to the current approach, but I'm not happy as operations like deletion, etc are not easily integrated

Would you be willing to talk to me regarding your choice on expanding this interface ?

SorsOps avatar Aug 10 '22 08:08 SorsOps

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
figma-tokens ✅ Ready (Inspect) Visit Preview Oct 28, 2022 at 0:37AM (UTC)
ft-storybook ✅ Ready (Inspect) Visit Preview Oct 28, 2022 at 0:37AM (UTC)

vercel[bot] avatar Aug 10 '22 08:08 vercel[bot]

This is awesome! 🔥 Let's schedule something for next week? This week is going to be still busy and we're about to merge the release-114 branch which contains quite a few changes (probably also affecting some parts of this PR), but happy to jump on a call to discuss this, I think this would greatly benefit everyone!

Can you DM me on twitter to discuss details?

six7 avatar Aug 11 '22 06:08 six7

@LiamMartens Would you mind giving some feedback on changing the interface ?

SorsOps avatar Aug 19 '22 12:08 SorsOps

@LiamMartens bump

SorsOps avatar Aug 23 '22 08:08 SorsOps

@andrewx82 sorry i havent had time yet, I'm going to dive in asap

LiamMartens avatar Aug 23 '22 08:08 LiamMartens

Hi @LiamMartens any update? I know Jan mentioned he would prefer to use the remote URL storage provider to be the base of the this storage system. In the case that we establish a protocol of the interaction between the remote and local with support for branches, etc should we also include a basic express server to demo how it would work?

SorsOps avatar Sep 01 '22 09:09 SorsOps

@LiamMartens pinging again, you mentioned you left some comments but I guess those are still in pending state?

six7 avatar Sep 04 '22 16:09 six7

@andrewx82 @six7 sorry I forgot to submit the comments...

LiamMartens avatar Sep 04 '22 16:09 LiamMartens

Thank you for the feedback @LiamMartens . Can I confirm with you that I should make the following changes to make the MR acceptable for you ?

  • Add the utility function for saveLastSyncedState
  • Eventlike support for the onChange event and the corresponding change for cloning. In this scenario I think it would be preferable that I pass through the entire array as you've suggested for now .

I'll obviously resolve the merge conflicts as well.

and then I just want to double check if you think the flow of POST to establish the endpoint and then PUTs and GETs afterwards for writes and reads respectively is fine as an interface.

Lastly, this obviously is for the simplified use case of having a static API key, do you have plans to support more complicated auth flows in the future?

SorsOps avatar Sep 05 '22 07:09 SorsOps

@andrewx82 sounds good; just some notes on your questions --

For the POST/PUT "flow" ; I wonder maybe if we need to make this configurable to some extent? I'm thinking very simply providing a few different modes:

  • Create mode ; uses POST to create/check for the existence first and PUT to write
  • Write mode ; uses POST simply to push changes whether they can be accepted or not
  • Read mode? ; only support reading from the remote resource (ie the basic URL storage we have now) --- happy for you to chip in here @six7

As for the authentication - chances are slim any more complex methods of auth will be supported in the near future. I assume you might be referring to OAuth like flows, but due to the Figma iframe these are very difficult to implement.

LiamMartens avatar Sep 07 '22 01:09 LiamMartens

Apologies for the delay, I have had to deal with some other work which blocked this. I will resolve all the above asap

SorsOps avatar Sep 14 '22 08:09 SorsOps