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

URL sync doesn't have support for $themes object #964

Closed
shedali opened this issue Jul 4, 2022 · 5 comments
Closed

URL sync doesn't have support for $themes object #964

shedali opened this issue Jul 4, 2022 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@shedali
Copy link

shedali commented Jul 4, 2022

attempted to use URL to a gist
https://gist.githubusercontent.com/shedali/034aa26437232a6fc7b9ec4a2d5cdc99/raw/92e0f0cd45309c89a3a4a04e18bff7f525f8805d/design-tokens.json

Receive error in plugin
image

However the network tab shows a 200 success response with the full payload.

Similarly using jsonbin as endpoint in the url sync tab
headers: { "X-Master-Key":"$2b$10$a53jnqUhVtfp9cSLHZEy9ullNNRg3REVHGqPkwqnmhm6D8br/5Enu", "X-Access-Key":"$2b$10$h.j7MhvlVb4XmaZuGJDqSulPb4xPifIlOiQcqaHLr9BNY0tHmC.5G " }
url: https://api.jsonbin.io/v3/b/62a9df97402a5b3802291976

@six7
Copy link
Collaborator

six7 commented Jul 6, 2022

Thanks for reporting!

The duplication error is already solved in the next branch which will be released soon.

As for the other issue: I discovered that this has to do with the empty $themes object. Can you remove that $themes key? We'll work on enabling support for that, I'll use this issue to track the work on that.

@LiamMartens This seems to only affect URL storage where a $themes object is present. I noticed our validationSchema doesn't yet include a $themes part in the zod validation.

@six7 six7 changed the title url sync does not work URL sync doesn't have support for $themes object Jul 6, 2022
@six7 six7 added the bug Something isn't working label Jul 6, 2022
@LiamMartens
Copy link
Contributor

@six7 this may already be fixed, because the singleFileSchema contains the $themes object already. Did we have the example JSON they used?

@six7
Copy link
Collaborator

six7 commented Jul 20, 2022

@six7 this may already be fixed, because the singleFileSchema contains the $themes object already. Did we have the example JSON they used?

The problem lies in the validation of URLTokenStorage when it's using the format without the values prop. If no $themes key exists, it works. If it exists, it doesn't (because it doesn't expect it).

Try to add the following URLs to URL sync:

This URL works: https://raw.githubusercontent.com/six7/tokens-storage/main/without-themes.json
This URL doesn't: https://raw.githubusercontent.com/six7/tokens-storage/main/with-themes.json

When console logging the validation it shows that zod is expecting an object, but the $themes property is an array.
image

https://github.com/six7/figma-tokens/blob/3535addeb74df37d054b0aae6a77ac10c5f444bf/src/storage/UrlTokenStorage.ts#L65

@LiamMartens
Copy link
Contributor

@six7 to be fair I am not even sure if we should support the older structure because it is too confusing from a file format perspective. It gets too complicated to mix keys like that in my opinion

@six7
Copy link
Collaborator

six7 commented Jul 20, 2022

@six7 to be fair I am not even sure if we should support the older structure because it is too confusing from a file format perspective. It gets too complicated to mix keys like that in my opinion

We'll need to support it somehow. I can imagine an option in the url storage modal asking what type of file this is (jsonbin format or regular)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants