Add DocumentState to the file Spec
Closed this issue · 20 comments
This is a new property on document which will be added by https://github.com/sketch-hq/Sketch/issues/30429
At the moment the only thing this property will contain is information on the cloud share - it'll be empty for normal documents. The contents of cloudShare are the same as that currently written to the User.json file under document/cloudShare. The idea is to remove this information from User.json at some point, but it's still there as well for backward compatibility.
"documentState" : {
"cloudShare" : {
"viewerCanUpdate" : 0,
"id" : "615164b7-1b51-49e4-be08-33aedabc8150",
"updatedAt" : "2020-06-17T14:54:49.378Z",
"owner" : {
"id" : "fad05bfc-1c06-41f4-9e91-39eb5df06765",
"__typename" : "PublicUser",
"updatedAt" : "2020-06-17T14:54:12.899Z",
"createdAt" : "2020-06-17T14:54:12.899Z"
},
"publicUrl" : "https:\/\/staging.sketch.cloud\/s\/615164b7-1b51-49e4-be08-33aedabc8150",
"shortId" : "615164b7-1b51-49e4-be08-33aedabc8150",
"isPrivate" : 1,
"currentVersion" : {
"id" : "b9f12673-44eb-4fb9-bcfe-9336cec237df",
"updatedAt" : "2020-06-17T14:54:49.378Z",
"createdAt" : "2020-06-17T14:54:49.378Z"
},
"createdAt" : "2020-06-17T14:54:49.378Z",
"name" : "Sketch Document",
"libraryEnabled" : 0
}
},
\cc @jedrichards - please ping me if you have any questions.
@opsGavin Will this have an associated document version bump?
I've updated the version in MSDocumentVersion.h if that's what you mean.
Yeap indeed. Since the document version is serialised to disk in Sketch files, it's also part of these schemas.
I see its being increased to 132 as part of this change 👍
@jedrichards might be worth to include a note on the User.json about its upcoming removal?
The cloudShare property is not currently included in the user schema anyway, this was the first I knew of it 🙈
That's probably ok TBH. It should only be on cloud documents and those should be hidden away in the app support folder. Plus - we really don't want users fiddling with this stuff.
we really don't want users fiddling with this stuff.
Let's not forget we are users too 😇
That's fine - we don't want you guys fiddling with it either 😝
Joking aside if you ever do see this information (like when running the assistant stuff) that might indicate an issue. If you do it would be good to know about it.
After a chat with @opsGavin, on the basis that cloudShare will never be visible in normal documents, and will never be visible to Assistants, it doesn't make sense to include it in the public file format, at this stage anyway. So to support 132 I'm just going to add the documentState property as an empty object. As I understand it, properties will be moving to it from user.json in future versions.
@jedrichards @opsGavin since it contains Cloud (share) information only, would it make sense to include it was workspace/cloud.json instead? Like other workspace objects, the structure would be seen as "out of scope" of the actual document spec.
Not sure. I was talking to Jed about the workspace stuff. I'm not altogether sure what's in there, but I think we'll need to figure out how to deal with it for collaboration.
@christianklotz @opsGavin There's an open PR for this #100
What's the status of this change Sketch-side? A document version bump to 132 is included in the above PR, so wondering if this has been implemented yet? If Sketch is not on 132 should we have seen Jenkins failures?
Well it's been stuck in CR / QA for about three weeks, and Jenkins has been randomly barfing at it 😣. BUT I think it's finally in the merge queue. All being well it should be merged today 🤞
Looks good to me @jedrichards 👍
Also bonus points for ephemeral - what a perfect description.
@opsGavin Ok that has been released as version 3.6.3. So to stop Jenkins complaining you can set in Sketch by following the instructions.
ey @opsGavin @jedrichards , reviewing this, one question arises. Why do we need so much information from the cloud stored here? It seems like the shareID and maybe the URL for convenience should be more than enough, right? /cc @dorianamouroux
Looking at the code change we didn't introduce any information from the Cloud, rather just specified a generic object called DocumentState with a vague description
Container for ephemeral document state. For now this is just a placeholder, and will see additions in future document versions.
The motivation at the time was just to accurately specify the file format both for purposes of internal knowledge sharing, and make the Sketch Assistants linters more robust.
I'm not sure why we store so much info here - that's historical from the original Cloud project - but this issue was originally really just about moving the information out of User.json and into the document.json.
Internally the idea with "document state" is that it stores information about the document which isn't altered as the document is edited. E.g. if you make a change and then undo, any cloud information shouldn't change. Likewise if you undo a change you still want to store the latest information provided by the Sketch Assistants.
But going back to the original change, the rational was that User.json contained metadata about the user's current state of the document (scroll position, etc). If this information was lost it wouldn't really matter. On the other hand, in collab this cloud information would be critical to the state of the document. For example, knowing the last patch which was applied is vital to ensuring that the document opens in a consistent state.
The eventual goal with this was that for cloud documents User.json would only live on the client machine. That would let individual users close their documents and re-open them in the same place they were at the last save. However, if a user downloaded a copy of the document it would contain the cloud information in document.json and would be able to sync when it opened (at the time the intention was that a downloaded document would open in collab mode).
FWIW I think there's probably some tidy up work to be done here - we kept the info in User.json to maintain backwards compatibility for a few releases, but I don't believe we ever went back and removed it. I think we planned to come back to it post release, but that never happened.
This slack discussion would also give you some context.