diff --git a/editor/src/components/editor/action-types.ts b/editor/src/components/editor/action-types.ts index ac27122d3015..ba0ef5a8db87 100644 --- a/editor/src/components/editor/action-types.ts +++ b/editor/src/components/editor/action-types.ts @@ -607,6 +607,7 @@ export interface UpdateFile { filePath: string file: ProjectFile addIfNotInFiles: boolean + fromCollaboration: boolean } export interface UpdateProjectContents { diff --git a/editor/src/components/editor/actions/action-creators.ts b/editor/src/components/editor/actions/action-creators.ts index 0f544841e60c..752bed846a11 100644 --- a/editor/src/components/editor/actions/action-creators.ts +++ b/editor/src/components/editor/actions/action-creators.ts @@ -1003,6 +1003,21 @@ export function updateFile( filePath: filePath, file: file, addIfNotInFiles: addIfNotInFiles, + fromCollaboration: false, + } +} + +export function updateFileFromCollaboration( + filePath: string, + file: ProjectFile, + addIfNotInFiles: boolean, +): UpdateFile { + return { + action: 'UPDATE_FILE', + filePath: filePath, + file: file, + addIfNotInFiles: addIfNotInFiles, + fromCollaboration: true, } } diff --git a/editor/src/components/editor/store/collaborative-editing.spec.ts b/editor/src/components/editor/store/collaborative-editing.spec.ts index 025cbe7746f4..edf6585dbd68 100644 --- a/editor/src/components/editor/store/collaborative-editing.spec.ts +++ b/editor/src/components/editor/store/collaborative-editing.spec.ts @@ -2,6 +2,8 @@ import { setFeatureForUnitTestsUseInDescribeBlockOnly, wait } from '../../../uti import { Y } from '../../../core/shared/yjs' import { RevisionsState, + assetFile, + imageFile, isParseSuccess, textFile, textFileContents, @@ -20,6 +22,8 @@ import { testParseCode } from '../../../core/workers/parser-printer/parser-print import { deleteFileFromCollaboration, updateExportsDetailFromCollaborationUpdate, + updateFile, + updateFileFromCollaboration, updateImportsFromCollaborationUpdate, updateTopLevelElementsFromCollaborationUpdate, } from '../actions/action-creators' @@ -75,7 +79,7 @@ async function runAddHookForProjectChangesTest(test: AddHookForProjectChangesTes Y.applyUpdate(secondCollaborationSession.mergeDoc, update) }) // Updates to the second doc should be applied to the first doc. - // Currently this includes updates from everywhere, including itsef. + // Currently this includes updates from everywhere, including itself. secondCollaborationSession.mergeDoc.on('update', (update) => { Y.applyUpdate(firstCollaborationSession.mergeDoc, update) }) @@ -147,6 +151,77 @@ describe('addHookForProjectChanges', () => { }) }) + it('adding image causes it to be added to the project contents', async () => { + const newFile = imageFile('jpg', undefined, 640, 480, 58345987345, undefined) + await runAddHookForProjectChangesTest({ + initialState: {}, + changesToApply: (firstSession, secondSession) => { + const updatedProjectContents = addFileToProjectContents({}, '/assets/test1.jpg', newFile) + const collaborationChanges = collateCollaborativeProjectChanges({}, updatedProjectContents) + updateCollaborativeProjectContents(firstSession, collaborationChanges, []) + }, + expectations: (firstSessionPromises, secondSessionPromises) => { + expect(secondSessionPromises).toEqual([ + updateFileFromCollaboration('/assets/test1.jpg', newFile, true), + ]) + }, + }) + }) + + it('adding image does not get added to the project contents if it includes the binary contents', async () => { + const newFile = imageFile( + 'jpg', + '84395834543hj5h4kjh5j43h5dfgdfgd', + 640, + 480, + 58345987345, + undefined, + ) + await runAddHookForProjectChangesTest({ + initialState: {}, + changesToApply: (firstSession, secondSession) => { + const updatedProjectContents = addFileToProjectContents({}, '/assets/test1.jpg', newFile) + const collaborationChanges = collateCollaborativeProjectChanges({}, updatedProjectContents) + updateCollaborativeProjectContents(firstSession, collaborationChanges, []) + }, + expectations: (firstSessionPromises, secondSessionPromises) => { + expect(secondSessionPromises).toEqual([]) + }, + }) + }) + + it('adding asset causes it to be added to the project contents', async () => { + const newFile = assetFile(undefined, '01189998819991197253') + await runAddHookForProjectChangesTest({ + initialState: {}, + changesToApply: (firstSession, secondSession) => { + const updatedProjectContents = addFileToProjectContents({}, '/assets/test1.ttf', newFile) + const collaborationChanges = collateCollaborativeProjectChanges({}, updatedProjectContents) + updateCollaborativeProjectContents(firstSession, collaborationChanges, []) + }, + expectations: (firstSessionPromises, secondSessionPromises) => { + expect(secondSessionPromises).toEqual([ + updateFileFromCollaboration('/assets/test1.ttf', newFile, true), + ]) + }, + }) + }) + + it('adding asset does not get added to the project contents if it includes the binary contents', async () => { + const newFile = assetFile('aaaaaaaabbbbbbbcccccccccccc', '01189998819991197253') + await runAddHookForProjectChangesTest({ + initialState: {}, + changesToApply: (firstSession, secondSession) => { + const updatedProjectContents = addFileToProjectContents({}, '/assets/test1.ttf', newFile) + const collaborationChanges = collateCollaborativeProjectChanges({}, updatedProjectContents) + updateCollaborativeProjectContents(firstSession, collaborationChanges, []) + }, + expectations: (firstSessionPromises, secondSessionPromises) => { + expect(secondSessionPromises).toEqual([]) + }, + }) + }) + it('adding file causes it to be added to the project contents, unless the file was modified by another user', async () => { const code = 'export const A =
' const newParsedCode = testParseCode(code) diff --git a/editor/src/components/editor/store/collaborative-editing.ts b/editor/src/components/editor/store/collaborative-editing.ts index 1011b40ac982..696047d3d6f5 100644 --- a/editor/src/components/editor/store/collaborative-editing.ts +++ b/editor/src/components/editor/store/collaborative-editing.ts @@ -2,10 +2,14 @@ import { deleteFileFromCollaboration, updateCodeFromCollaborationUpdate, updateExportsDetailFromCollaborationUpdate, + updateFile, + updateFileFromCollaboration, updateImportsFromCollaborationUpdate, } from '../actions/action-creators' import type { + CollabAssetFile, CollabFile, + CollabImageFile, CollabTextFile, CollabTextFileExportsDetail, CollabTextFileImports, @@ -25,11 +29,13 @@ import { isLoggedIn, type EditorAction, type EditorDispatch } from '../action-ty import { updateTopLevelElementsFromCollaborationUpdate } from '../actions/action-creators' import { assertNever } from '../../../core/shared/utils' import type { + AssetFile, ExportDetail, + ImageFile, ImportDetails, ParseSuccess, } from '../../../core/shared/project-file-types' -import { isTextFile } from '../../../core/shared/project-file-types' +import { assetFile, imageFile, isTextFile } from '../../../core/shared/project-file-types' import { ExportDetailKeepDeepEquality, ImportDetailsKeepDeepEquality, @@ -50,6 +56,7 @@ import type { MapLike } from 'typescript' import { Y } from '../../../core/shared/yjs' import type { ProjectServerState } from './project-server-state' import { Substores, useEditorState } from './store-hook' +import { forceNotNull } from '../../../core/shared/optional-utils' const CodeKey = 'code' const TopLevelElementsKey = 'topLevelElements' @@ -183,18 +190,44 @@ function applyFileChangeToMap( } break case 'WRITE_PROJECT_FILE': - if ( - change.projectFile.type === 'TEXT_FILE' && - looksLikeParseableSourceCode(change.fullPath) && - change.projectFile.fileContents.parsed.type === 'PARSE_SUCCESS' - ) { - updateFromParseSuccess(projectContentsMap, change, change.projectFile.fileContents.parsed) - } - if ( - change.projectFile.type === 'TEXT_FILE' && - !looksLikeParseableSourceCode(change.fullPath) - ) { - updateNonParseable(projectContentsMap, change, change.projectFile.fileContents.code) + switch (change.projectFile.type) { + case 'TEXT_FILE': + { + if (looksLikeParseableSourceCode(change.fullPath)) { + if (change.projectFile.fileContents.parsed.type === 'PARSE_SUCCESS') { + updateFromParseSuccess( + projectContentsMap, + change, + change.projectFile.fileContents.parsed, + ) + } + } else { + updateNonParseable(projectContentsMap, change, change.projectFile.fileContents.code) + } + } + break + case 'IMAGE_FILE': + { + // Should avoid shunting the base64 blobs across the wire, + // especially as that's a transient state during the upload of the image. + if (change.projectFile.base64 == null) { + updateImageFile(projectContentsMap, change, change.projectFile) + } + } + break + + case 'ASSET_FILE': + { + // Should avoid shunting the base64 blobs across the wire, + // especially as that's a transient state during the upload of the asset. + if (change.projectFile.base64 == null) { + updateAssetFile(projectContentsMap, change, change.projectFile) + } + } + break + case 'DIRECTORY': + // Do nothing. + break } break case 'ENSURE_DIRECTORY_EXISTS': @@ -205,47 +238,77 @@ function applyFileChangeToMap( } } +function clearIfNotExpectedType( + projectContentsMap: Y.Map, + filename: string, + expectedType: string, +): T { + let result: Y.Map + if (projectContentsMap.has(filename)) { + result = projectContentsMap.get(filename)! + if (result.get('type') !== expectedType) { + result.clear() + result.set('type', expectedType) + } + } else { + result = new Y.Map() + result.set('type', expectedType) + projectContentsMap.set(filename, result) + } + return result as T +} + +function updateImageFile( + projectContentsMap: Y.Map, + change: WriteProjectFileChange, + imageFileForUpdate: ImageFile, +): void { + projectContentsMap.set(change.fullPath, new Y.Map(Object.entries(imageFileForUpdate))) +} + +function updateAssetFile( + projectContentsMap: Y.Map, + change: WriteProjectFileChange, + assetFileForUpdate: AssetFile, +): void { + projectContentsMap.set(change.fullPath, new Y.Map(Object.entries(assetFileForUpdate))) +} + function updateNonParseable( - projectContentsMap: Y.Map, + projectContentsMap: Y.Map, change: WriteProjectFileChange, code: string, ): void { - let collabFile: CollabTextFile - if (projectContentsMap.has(change.fullPath)) { - collabFile = projectContentsMap.get(change.fullPath)! - } else { - collabFile = new Y.Map() - projectContentsMap.set(change.fullPath, collabFile) - } - collabFile.set('type', 'TEXT_FILE') - collabFile.delete(TopLevelElementsKey) - collabFile.delete(ExportsDetailKey) - collabFile.delete(ImportsKey) - collabFile.set(CodeKey, code) + const collabTextFile: CollabTextFile = clearIfNotExpectedType( + projectContentsMap, + change.fullPath, + 'TEXT_FILE', + ) + collabTextFile.delete(TopLevelElementsKey) + collabTextFile.delete(ExportsDetailKey) + collabTextFile.delete(ImportsKey) + collabTextFile.set(CodeKey, code) } function updateFromParseSuccess( - projectContentsMap: Y.Map, + projectContentsMap: Y.Map, change: WriteProjectFileChange, parsedPart: ParseSuccess, ): void { - let collabFile: CollabTextFile - if (projectContentsMap.has(change.fullPath)) { - collabFile = projectContentsMap.get(change.fullPath)! - } else { - collabFile = new Y.Map() - projectContentsMap.set(change.fullPath, collabFile) - } - collabFile.set('type', 'TEXT_FILE') + const collabTextFile: CollabTextFile = clearIfNotExpectedType( + projectContentsMap, + change.fullPath, + 'TEXT_FILE', + ) const topLevelElementsArray = new Y.Array() - collabFile.set(TopLevelElementsKey, topLevelElementsArray) + collabTextFile.set(TopLevelElementsKey, topLevelElementsArray) const exportsDetailArray = new Y.Array() - collabFile.set(ExportsDetailKey, exportsDetailArray) + collabTextFile.set(ExportsDetailKey, exportsDetailArray) const importsMap = new Y.Map() - collabFile.set(ImportsKey, importsMap) - collabFile.delete(CodeKey) + collabTextFile.set(ImportsKey, importsMap) + collabTextFile.delete(CodeKey) - synchroniseParseSuccessToCollabFile(parsedPart, collabFile) + synchroniseParseSuccessToCollabFile(parsedPart, collabTextFile) } export function updateCollaborativeProjectContents( @@ -308,9 +371,25 @@ export function addHookForProjectChanges( const filePath = changeEvent.path[0] as string if (changeEvent instanceof Y.YMapEvent) { for (const key of changeEvent.keysChanged) { - if (key !== 'type') { - if (changeEvent.target.has(key)) { - fileAndPropertyUpdate(filePath, key) + switch (changeEvent.target.get('type')) { + case 'TEXT_FILE': { + if (key !== 'type') { + if (changeEvent.target.has(key)) { + fileAndPropertyUpdate(filePath, key) + } + } + break + } + case 'IMAGE_FILE': { + actionsToDispatch.push(updateImageActionFromSession(session, filePath)) + break + } + case 'ASSET_FILE': { + actionsToDispatch.push(updateAssetActionFromSession(session, filePath)) + break + } + default: { + throw new Error(`Unexpected file type: ${changeEvent.target.get('type')}`) } } } @@ -341,46 +420,79 @@ function updateEntireProjectContents(changeEvent: Y.YMapEvent): Array for (const [filename, change] of changeEvent.keys.entries()) { - switch (change.action) { - case 'delete': - actions.push(deleteFileFromCollaboration(filename)) - break - case 'add': - case 'update': - // Mysteriously the type doesn't really carry over. - const entryFile = targetMap.get(filename) as CollabFile - // Handle `topLevelElements`. - const topLevelElements = entryFile.get(TopLevelElementsKey) as - | CollabTextFileTopLevelElements - | undefined - if (topLevelElements != null) { - actions.push( - updateTopLevelElementsFromCollaborationUpdate(filename, topLevelElements.toArray()), - ) - } - // Handle `exportsDetail`. - const exportsDetail = entryFile.get(ExportsDetailKey) as - | CollabTextFileExportsDetail - | undefined - if (exportsDetail != null) { - actions.push( - updateExportsDetailFromCollaborationUpdate(filename, exportsDetail.toArray()), - ) + if (change.action === 'delete') { + // Handle deletion separately as `targetMap` cannot be usefully + // queried for these cases. + actions.push(deleteFileFromCollaboration(filename)) + } else { + switch (targetMap.get(filename)?.get('type')) { + case 'TEXT_FILE': { + switch (change.action) { + case 'add': + case 'update': + // Mysteriously the type doesn't really carry over. + const entryFile = targetMap.get(filename) as CollabTextFile + // Handle `topLevelElements`. + const topLevelElements = entryFile.get(TopLevelElementsKey) as + | CollabTextFileTopLevelElements + | undefined + if (topLevelElements != null) { + actions.push( + updateTopLevelElementsFromCollaborationUpdate( + filename, + topLevelElements.toArray(), + ), + ) + } + // Handle `exportsDetail`. + const exportsDetail = entryFile.get(ExportsDetailKey) as + | CollabTextFileExportsDetail + | undefined + if (exportsDetail != null) { + actions.push( + updateExportsDetailFromCollaborationUpdate(filename, exportsDetail.toArray()), + ) + } + // Handle `imports`. + const imports = entryFile.get(ImportsKey) as CollabTextFileImports | undefined + if (imports != null) { + actions.push(updateImportsFromCollaborationUpdate(filename, imports.toJSON())) + } + + // Handle `code`. + const code = entryFile.get(CodeKey) as string | undefined + if (code != null) { + actions.push(updateCodeFromCollaborationUpdate(filename, code)) + } + break + default: + assertNever(change.action) + } + break } - // Handle `imports`. - const imports = entryFile.get(ImportsKey) as CollabTextFileImports | undefined - if (imports != null) { - actions.push(updateImportsFromCollaborationUpdate(filename, imports.toJSON())) + case 'IMAGE_FILE': { + switch (change.action) { + case 'add': + case 'update': + // Mysteriously the type doesn't really carry over. + const entryFile = targetMap.get(filename) as CollabImageFile + actions.push(updateImageActionFromFile(entryFile, filename)) + } + break } - - // Handle `code`. - const code = entryFile.get(CodeKey) as string | undefined - if (code != null) { - actions.push(updateCodeFromCollaborationUpdate(filename, code)) + case 'ASSET_FILE': { + switch (change.action) { + case 'add': + case 'update': + // Mysteriously the type doesn't really carry over. + const entryFile = targetMap.get(filename) as CollabAssetFile + actions.push(updateAssetActionFromFile(entryFile, filename)) + } + break } - break - default: - assertNever(change.action) + default: + // Ignore for now, directories potentially. + } } } @@ -459,6 +571,51 @@ function updateImportsOfFile( ) } +function updateImageActionFromFile(file: CollabImageFile, filePath: string): EditorAction { + return updateFileFromCollaboration( + filePath, + imageFile( + file.get('imageType') as string | undefined, + undefined, + file.get('width') as number | undefined, + file.get('height') as number | undefined, + file.get('hash') as number, + file.get('gitBlobSha') as string | undefined, + ), + true, + ) +} + +function updateImageActionFromSession( + session: CollaborativeEditingSupportSession, + filePath: string, +): EditorAction { + const file = forceNotNull( + 'File should exist.', + session.projectContents.get(filePath), + ) as CollabImageFile + return updateImageActionFromFile(file, filePath) +} + +function updateAssetActionFromFile(file: CollabAssetFile, filePath: string): EditorAction { + return updateFileFromCollaboration( + filePath, + assetFile(undefined, file.get('gitBlobSha') as string | undefined), + true, + ) +} + +function updateAssetActionFromSession( + session: CollaborativeEditingSupportSession, + filePath: string, +): EditorAction { + const file = forceNotNull( + 'File should exist.', + session.projectContents.get(filePath), + ) as CollabAssetFile + return updateAssetActionFromFile(file, filePath) +} + function updateCodeOfFile( session: CollaborativeEditingSupportSession, filePath: string, diff --git a/editor/src/components/editor/store/editor-state.ts b/editor/src/components/editor/store/editor-state.ts index f37b2758c9d5..57147b23c29b 100644 --- a/editor/src/components/editor/store/editor-state.ts +++ b/editor/src/components/editor/store/editor-state.ts @@ -418,7 +418,11 @@ export type CollabTextFile = Y.Map< | string > -export type CollabFile = CollabTextFile //| CollabImageFileUpdate | CollabAssetFileUpdate | CollabDirectoryFileUpdate +export type CollabImageFile = Y.Map<'IMAGE_FILE' | string | number> + +export type CollabAssetFile = Y.Map<'ASSET_FILE' | string> + +export type CollabFile = CollabTextFile | CollabImageFile | CollabAssetFile export interface CollaborativeEditingSupportSession { mergeDoc: Y.Doc diff --git a/editor/src/components/editor/store/editor-update.tsx b/editor/src/components/editor/store/editor-update.tsx index 0f8629f319a3..cffdebd951f0 100644 --- a/editor/src/components/editor/store/editor-update.tsx +++ b/editor/src/components/editor/store/editor-update.tsx @@ -83,6 +83,9 @@ export function gatedActions( case 'DELETE_FILE_FROM_COLLABORATION': isCollaborationUpdate = true break + case 'UPDATE_FILE': + isCollaborationUpdate = action.fromCollaboration + break case 'UPDATE_FROM_WORKER': alwaysRun = true break diff --git a/editor/src/core/model/project-file-utils.ts b/editor/src/core/model/project-file-utils.ts index e1f2389dad9b..24e0d8d224f2 100644 --- a/editor/src/core/model/project-file-utils.ts +++ b/editor/src/core/model/project-file-utils.ts @@ -30,6 +30,8 @@ import { isExportDefaultFunctionOrClass, isExportFunction, isExportDefault, + isImageFile, + isAssetFile, } from '../shared/project-file-types' import type { JSXElementChild, @@ -75,6 +77,10 @@ import { memoize } from '../shared/memoize' import { filenameFromParts, getFilenameParts } from '../../components/images' import globToRegexp from 'glob-to-regexp' import { is } from '../shared/equality-utils' +import { + AssetFileKeepDeepEquality, + ImageFileKeepDeepEquality, +} from '../../components/editor/store/store-deep-equality-instances' export const sceneMetadata = _sceneMetadata // This is a hotfix for a circular dependency AND a leaking of utopia-api into the workers @@ -535,12 +541,13 @@ export function updateFileIfPossible( updated: ProjectFile, existing: ProjectFile | null, ): ProjectFile | 'cant-update' { - if (existing == null || !isTextFile(existing)) { + if (existing == null || existing.type !== updated.type) { return updated } if ( isTextFile(updated) && + isTextFile(existing) && isOlderThan(existing, updated) && canUpdateRevisionsState( updated.fileContents.revisionsState, @@ -550,6 +557,18 @@ export function updateFileIfPossible( return updated } + if (isImageFile(updated) && existing != null && isImageFile(existing)) { + if (ImageFileKeepDeepEquality(existing, updated).areEqual) { + return 'cant-update' + } + } + + if (isAssetFile(updated) && existing != null && isAssetFile(existing)) { + if (AssetFileKeepDeepEquality(existing, updated).areEqual) { + return 'cant-update' + } + } + return 'cant-update' }