From 732745c8371c3508dd6080f801392733663d1dba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20GS=20Pereira?= Date: Thu, 6 Aug 2026 16:26:37 -0300 Subject: [PATCH 1/2] feat(data-types): add a form/code toggle to the data type editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every data type tab gains the variables-style table/Monaco switch, with the code side showing that type's `.dt` text (DOPE-535). Committing the buffer parses it through `parseDataTypeFromText`, so an invalid declaration keeps the user on the text instead of silently dropping the edit, and renaming in the text stays rejected — the file name is the type's identity. `StructureTableType` becomes the same discriminated union the variables table uses, which is why `updateModelStructure` grows `display`/`code` and gains a name-scoped sibling: every open data type is mounted at once, so a background editor writing through the active-editor action would land on the wrong model. `display` is optional there on purpose — a row selection must never flip the view. The reconcile/regenerate pair keeps the buffer and the store in lockstep when something outside the code view moves the type. A tree rename folds pending text edits in first and refuses on invalid text, rather than regenerating over work the user hasn't committed; undo, redo and a disk revert regenerate so Monaco shows the restored state. An unreadable `datatypes/.dt` has no `PLCDataType`, so it can't appear in the project tree and until now had no way of being found or fixed in-app. Project open now registers it and pre-opens a code-mode tab holding the raw bytes, without stealing focus from the auto-opened POU; committing valid text promotes it to a real type. The store can't raise a toast (layer rule), so the parse warning still goes to the console. Gated by `isDataTypeFilesEnabled()`, which ships false — DOPE-542 owns the flip. Refs DOPE-535 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GvGrhB1LyJz9MBwYhjoBDY --- .../_features/[workspace]/data-type/index.tsx | 234 +++++++++++++++++- .../_molecules/data-types/structure/index.tsx | 12 +- .../store/__tests__/editor-slice.test.ts | 81 +++++- .../store/__tests__/project-slice.test.ts | 112 +++++++++ .../store/__tests__/shared-slice.test.ts | 56 +++++ .../store/__tests__/shared-utils.test.ts | 20 +- .../store/__tests__/tabs-utils.test.ts | 2 +- src/frontend/store/slices/editor/slice.ts | 37 ++- src/frontend/store/slices/editor/types.ts | 26 +- src/frontend/store/slices/project/slice.ts | 65 +++++ src/frontend/store/slices/project/types.ts | 7 + src/frontend/store/slices/shared/slice.ts | 46 +++- src/frontend/store/slices/shared/utils.ts | 12 +- src/frontend/store/slices/tabs/utils.ts | 2 +- 14 files changed, 678 insertions(+), 34 deletions(-) diff --git a/src/frontend/components/_features/[workspace]/data-type/index.tsx b/src/frontend/components/_features/[workspace]/data-type/index.tsx index a177ff0e6..fc46a36c0 100644 --- a/src/frontend/components/_features/[workspace]/data-type/index.tsx +++ b/src/frontend/components/_features/[workspace]/data-type/index.tsx @@ -1,12 +1,20 @@ -import { ComponentPropsWithoutRef, useEffect, useState } from 'react' +import { ComponentPropsWithoutRef, useEffect, useRef, useState } from 'react' import type { PLCDataType } from '../../../../../middleware/shared/ports/types' +import { CodeIcon } from '../../../../assets/icons/interface/CodeIcon' +import { TableIcon } from '../../../../assets/icons/interface/TableIcon' +import { usePouSnapshot } from '../../../../hooks/use-pou-snapshot' import { useOpenPLCStore } from '../../../../store' import { extractSearchQuery } from '../../../../store/slices/search/utils' +import { cn } from '../../../../utils/cn' +import { isDataTypeFilesEnabled } from '../../../../utils/feature-flags' +import { serializeDataTypeToText } from '../../../../utils/PLC/data-type-serializer' +import { parseDataTypeFromText } from '../../../../utils/PLC/data-type-text-parser' import { InputWithRef } from '../../../_atoms/input' import { ArrayDataType } from '../../../_molecules/data-types/array' import { EnumeratorDataType } from '../../../_molecules/data-types/enumerated' import { StructureDataType } from '../../../_molecules/data-types/structure' +import { VariablesCodeEditor } from '../../../_organisms/variables-code-editor' import { toast } from '../../[app]/toast/use-toast' type DatatypeEditorProps = ComponentPropsWithoutRef<'div'> & { @@ -15,23 +23,180 @@ type DatatypeEditorProps = ComponentPropsWithoutRef<'div'> & { const DataTypeEditor = ({ dataTypeName, ...rest }: DatatypeEditorProps) => { const { + editor, + editors, project: { data: { dataTypes }, }, + unparsedDataTypeFiles, + workspace: { + systemConfigs: { shouldUseDarkMode }, + }, datatypeActions: { rename }, + editorActions: { updateModelStructureForName }, + projectActions: { createDatatype, removeUnparsedDataTypeFile, updateDatatype }, + sharedWorkspaceActions: { handleFileAndWorkspaceSavedState }, searchQuery, } = useOpenPLCStore() + const { captureAndPush } = usePouSnapshot() + + // Every open data type is mounted at once (workspace-screen keeps the + // inactive ones hidden), so the view state has to come from this + // type's own model — never from the active `editor`. + const model = editor.meta.name === dataTypeName ? editor : editors.find((e) => e.meta.name === dataTypeName) + const modelStructure = model?.type === 'plc-datatype' ? model.structure : undefined + const codeViewEnabled = isDataTypeFilesEnabled() + const display = codeViewEnabled && modelStructure?.display === 'code' ? 'code' : 'table' + const modelCode = modelStructure?.display === 'code' ? modelStructure.code : undefined + + // A `.dt` file that failed to parse has no entry in `dataTypes`; the + // raw text is all there is until the user fixes it. + const rawFile = unparsedDataTypeFiles.find( + (file) => file.relativePath.split('/').pop()?.replace(/\.dt$/i, '') === dataTypeName, + ) + const [editorContent, setEditorContent] = useState() const [isEditing, setIsEditing] = useState(false) + const [editorCode, setEditorCode] = useState(() => { + if (typeof modelCode === 'string') return modelCode + const dataType = dataTypes.find((candidate) => candidate.name === dataTypeName) + return dataType ? serializeDataTypeToText(dataType) : (rawFile?.content ?? '') + }) + const [parseError, setParseError] = useState(null) + + const containerRef = useRef(null) + const latestCodeRef = useRef(editorCode) + const latestDisplayRef = useRef(display) + const lastParsedCodeRef = useRef(editorCode) + const lastMirroredCodeRef = useRef(editorCode) + const isParsingRef = useRef(false) + const commitCodeRef = useRef<() => boolean>(() => false) useEffect(() => { - const dataTypeIndex = dataTypes.findIndex((dataType) => dataType.name === dataTypeName) - if (dataTypeIndex !== -1) { - const dataType = dataTypes[dataTypeIndex] - setEditorContent(dataType) - } + const dataType = dataTypes.find((candidate) => candidate.name === dataTypeName) + if (dataType) setEditorContent(dataType) }, [dataTypes, dataTypeName]) + // Keep the buffer serialized from the form while in table mode, so the + // toggle already holds the right text the moment it flips. + useEffect(() => { + if (display === 'code') return + const text = editorContent ? serializeDataTypeToText(editorContent) : (rawFile?.content ?? '') + setEditorCode(text) + // In table mode the form is the committed state, so this is also + // the watermark the next outside-click compares against. + lastParsedCodeRef.current = text + }, [editorContent, display, rawFile?.content]) + + // Adopt buffers written by the store (a tree rename or an undo + // regenerates them), but never the echo of our own mirror below — + // that would race the keystroke that produced it. + useEffect(() => { + if (display !== 'code' || typeof modelCode !== 'string') return + if (modelCode === lastMirroredCodeRef.current) return + setEditorCode(modelCode) + }, [display, modelCode]) + + useEffect(() => { + if (display !== 'code') return + lastMirroredCodeRef.current = editorCode + updateModelStructureForName(dataTypeName, { display: 'code', code: editorCode }) + }, [editorCode, display, dataTypeName, updateModelStructureForName]) + + useEffect(() => { + latestCodeRef.current = editorCode + latestDisplayRef.current = display + }, [editorCode, display]) + + useEffect(() => { + return () => { + if (latestDisplayRef.current === 'code') { + updateModelStructureForName(dataTypeName, { display: 'code', code: latestCodeRef.current }) + } + } + }, [dataTypeName, updateModelStructureForName]) + + // A type that doesn't exist yet is a broken file on disk: show why it + // is broken while the user edits, instead of only on commit. + useEffect(() => { + if (display !== 'code' || editorContent) return + setParseError(parseDataTypeFromText(editorCode, dataTypeName).error ?? null) + }, [display, editorContent, editorCode, dataTypeName]) + + const commitCode = (): boolean => { + const { dataType, error } = parseDataTypeFromText(editorCode, dataTypeName) + if (!dataType) { + const message = error ?? 'Unexpected syntax error.' + setParseError(message) + toast({ title: 'Syntax error', description: message, variant: 'fail' }) + return false + } + + captureAndPush(dataTypeName) + + if (editorContent) { + updateDatatype(dataTypeName, dataType) + } else { + const result = createDatatype({ data: dataType }) + if (!result.ok) { + const message = result.message ?? 'Could not create the data type.' + setParseError(message) + toast({ title: 'Syntax error', description: message, variant: 'fail' }) + return false + } + if (rawFile) removeUnparsedDataTypeFile(rawFile.relativePath) + } + + handleFileAndWorkspaceSavedState(dataTypeName) + setParseError(null) + return true + } + + useEffect(() => { + commitCodeRef.current = commitCode + }) + + useEffect(() => { + if (display !== 'code') return + + const tryCommit = () => { + if (isParsingRef.current) return + if (editorCode === lastParsedCodeRef.current) return + isParsingRef.current = true + if (commitCodeRef.current()) lastParsedCodeRef.current = editorCode + isParsingRef.current = false + } + + const onDocMouseDown = (e: MouseEvent) => { + if (!containerRef.current) return + if (containerRef.current.contains(e.target as Node)) return + tryCommit() + } + + // Covers keyboard navigation, Tab and shortcuts — anything that + // moves focus away without a mousedown. + const onFocusOut = (e: FocusEvent) => { + if (!containerRef.current) return + const newTarget = e.relatedTarget as Node | null + if (newTarget && containerRef.current.contains(newTarget)) return + tryCommit() + } + + const container = containerRef.current + document.addEventListener('mousedown', onDocMouseDown, true) + container?.addEventListener('focusout', onFocusOut) + return () => { + document.removeEventListener('mousedown', onDocMouseDown, true) + container?.removeEventListener('focusout', onFocusOut) + } + }, [display, editorCode]) + + const handleVisualizationTypeChange = (value: 'code' | 'table') => { + if (display === value) return + if (display === 'code' && !commitCode()) return + updateModelStructureForName(dataTypeName, { display: value, code: value === 'code' ? editorCode : undefined }) + } + const handleStartEditing = () => { setIsEditing(true) } @@ -64,6 +229,7 @@ const DataTypeEditor = ({ dataTypeName, ...rest }: DatatypeEditorProps) => { return (
{ > {isEditing ? ( { aria-label='Data type name' className='h-full w-full bg-transparent p-2 text-start font-caption text-xs text-neutral-850 outline-none dark:text-neutral-100' onClick={handleStartEditing} - dangerouslySetInnerHTML={{ __html: extractSearchQuery(editorContent?.name || '', searchQuery) }} + dangerouslySetInnerHTML={{ + __html: extractSearchQuery(editorContent?.name ?? dataTypeName, searchQuery), + }} /> )}
+ {codeViewEnabled && ( +
+ handleVisualizationTypeChange('table')} + size='md' + currentVisible={display === 'table'} + className={cn( + display === 'table' ? 'fill-brand' : 'fill-neutral-100 dark:fill-neutral-900', + 'rounded-l-md transition-colors ease-in-out hover:cursor-pointer', + )} + /> + handleVisualizationTypeChange('code')} + size='md' + currentVisible={display === 'code'} + className={cn( + display === 'code' ? 'fill-brand' : 'fill-neutral-100 dark:fill-neutral-900', + 'rounded-r-md transition-colors ease-in-out hover:cursor-pointer', + )} + /> +
+ )} -
- {editorContent?.derivation === 'array' && } - {editorContent?.derivation === 'enumerated' && } - {editorContent?.derivation === 'structure' && } +
+ {display === 'table' ? ( + <> + {editorContent?.derivation === 'array' && } + {editorContent?.derivation === 'enumerated' && } + {editorContent?.derivation === 'structure' && } + + ) : ( + <> +
+ +
+ {parseError &&

Error: {parseError}

} + + )}
) diff --git a/src/frontend/components/_molecules/data-types/structure/index.tsx b/src/frontend/components/_molecules/data-types/structure/index.tsx index fd20b81c0..343f04b3e 100644 --- a/src/frontend/components/_molecules/data-types/structure/index.tsx +++ b/src/frontend/components/_molecules/data-types/structure/index.tsx @@ -28,7 +28,8 @@ const StructureDataType = () => { const [tableData, setTableData] = useState([]) - const [editorStructure, setEditorStructure] = useState({ + const [editorStructure, setEditorStructure] = useState>({ + display: 'table', selectedRow: ROWS_NOT_SELECTED.toString(), description: '', }) @@ -47,9 +48,14 @@ const StructureDataType = () => { useEffect(() => { const foundDataType = dataTypes.find((dataType) => dataType?.derivation === 'structure') - if (editor.type === 'plc-datatype' && foundDataType && 'variable' in foundDataType) { + if ( + editor.type === 'plc-datatype' && + editor.structure.display === 'table' && + foundDataType && + 'variable' in foundDataType + ) { const { description, selectedRow } = editor.structure - setEditorStructure({ description: description, selectedRow: selectedRow }) + setEditorStructure({ display: 'table', description: description, selectedRow: selectedRow }) } }, [editor]) diff --git a/src/frontend/store/__tests__/editor-slice.test.ts b/src/frontend/store/__tests__/editor-slice.test.ts index 4cf2c9e5f..8facf03dc 100644 --- a/src/frontend/store/__tests__/editor-slice.test.ts +++ b/src/frontend/store/__tests__/editor-slice.test.ts @@ -58,7 +58,7 @@ function makeDatatype(name: string): EditorModel { return { type: 'plc-datatype', meta: { name, derivation: 'structure' }, - structure: { selectedRow: '-1', description: '' }, + structure: { display: 'table', selectedRow: '-1', description: '' }, } } @@ -217,7 +217,11 @@ describe('editor slice', () => { a.setEditor(makeTextual('M')) a.updateModelVariablesForName('DT', { display: 'table', selectedRow: 1 }) const dtEditor = store.getState().editors.find((e) => e.meta.name === 'DT')! - expect(editorAs(dtEditor).structure.selectedRow).toBe('-1') + expect(editorAs(dtEditor).structure).toEqual({ + display: 'table', + selectedRow: '-1', + description: '', + }) }) }) @@ -281,12 +285,14 @@ describe('editor slice', () => { a.updateModelStructure({ selectedRow: 3, description: 'desc' }) expect(editorAs(store.getState().editor).structure).toEqual({ + display: 'table', selectedRow: '3', description: 'desc', }) // undefined selectedRow → keeps; empty description → keeps (falsy) a.updateModelStructure({ description: '' }) expect(editorAs(store.getState().editor).structure).toEqual({ + display: 'table', selectedRow: '3', description: 'desc', }) @@ -296,6 +302,77 @@ describe('editor slice', () => { store.getState().editorActions.updateModelStructure({ selectedRow: 1, description: 'x' }) expect(store.getState().editor.type).toBe('available') }) + + it('switches to code mode keeping the buffer, and back to table with defaults', () => { + const { editorActions: a } = store.getState() + const dt = makeDatatype('S') + a.addModel(dt) + a.setEditor(dt) + + a.updateModelStructure({ selectedRow: 2, description: 'desc' }) + a.updateModelStructure({ display: 'code', code: 'TYPE\nS : (A);\nEND_TYPE\n' }) + expect(editorAs(store.getState().editor).structure).toEqual({ + display: 'code', + code: 'TYPE\nS : (A);\nEND_TYPE\n', + }) + + // no `code` on a code→code update keeps the existing buffer + a.updateModelStructure({ display: 'code' }) + expect(editorAs(store.getState().editor).structure).toEqual({ + display: 'code', + code: 'TYPE\nS : (A);\nEND_TYPE\n', + }) + + // the table arm's fields don't survive a round trip through code mode + a.updateModelStructure({ display: 'table' }) + expect(editorAs(store.getState().editor).structure).toEqual({ + display: 'table', + selectedRow: '-1', + description: '', + }) + }) + }) + + describe('updateModelStructureForName', () => { + it('updates a non-active model', () => { + const { editorActions: a } = store.getState() + const active = makeDatatype('Active') + const background = makeDatatype('Background') + a.addModel(active) + a.addModel(background) + a.setEditor(active) + + a.updateModelStructureForName('Background', { display: 'code', code: 'raw' }) + + const stored = store.getState().editors.find((e) => e.meta.name === 'Background') + expect(stored && editorAs(stored).structure).toEqual({ display: 'code', code: 'raw' }) + expect(editorAs(store.getState().editor).structure).toEqual({ + display: 'table', + selectedRow: '-1', + description: '', + }) + }) + + it('updates the active model when the name matches', () => { + const { editorActions: a } = store.getState() + const dt = makeDatatype('S') + a.addModel(dt) + a.setEditor(dt) + + a.updateModelStructureForName('S', { display: 'code', code: 'raw' }) + expect(editorAs(store.getState().editor).structure).toEqual({ display: 'code', code: 'raw' }) + }) + + it('no-op for an unknown name or a non-datatype model', () => { + const { editorActions: a } = store.getState() + const textual = makeTextual('P') + a.addModel(textual) + a.setEditor(textual) + + a.updateModelStructureForName('missing', { display: 'code', code: 'raw' }) + a.updateModelStructureForName('P', { display: 'code', code: 'raw' }) + expect(store.getState().editor.type).toBe('plc-textual') + }) }) describe('updateModelLadder', () => { diff --git a/src/frontend/store/__tests__/project-slice.test.ts b/src/frontend/store/__tests__/project-slice.test.ts index a20a20d53..a034bd51a 100644 --- a/src/frontend/store/__tests__/project-slice.test.ts +++ b/src/frontend/store/__tests__/project-slice.test.ts @@ -21,6 +21,7 @@ import type { S7CommDataBlock, } from '../../../middleware/shared/ports/types' import { generateIecVariablesToString } from '../../utils/generate-iec-variables-to-string' +import { serializeDataTypeToText } from '../../utils/PLC/data-type-serializer' import { createConsoleSlice } from '../slices/console' import { createDeviceSlice } from '../slices/device' import { createEditorSlice } from '../slices/editor' @@ -47,6 +48,19 @@ function makeStore() { // Helpers // --------------------------------------------------------------------------- +function openDatatypeInCodeMode(store: ReturnType, name: string, code: string) { + store.getState().editorActions.addModel({ + type: 'plc-datatype', + meta: { name, derivation: 'enumerated' }, + structure: { display: 'code', code }, + }) +} + +function codeOf(store: ReturnType, name: string): string | undefined { + const model = store.getState().editors.find((e) => e.meta.name === name) + return model?.type === 'plc-datatype' && model.structure.display === 'code' ? model.structure.code : undefined +} + function makeVariable(name: string, cls: PLCVariable['class'] = 'local'): PLCVariable { return { name, @@ -1308,6 +1322,104 @@ describe('createProjectSlice', () => { store.getState().projectActions.applyDatatypeSnapshot('NonExistent', replacement) expect(store.getState().project.data.dataTypes[0].name).toBe('A') }) + + it('regenerates the code buffer of a type shown in code mode', () => { + const dt: PLCDataType = { name: 'A', derivation: 'enumerated', values: [{ description: 'RED' }] } + store.getState().projectActions.createDatatype({ data: dt }) + openDatatypeInCodeMode(store, 'A', 'stale text') + + store.getState().projectActions.applyDatatypeSnapshot('A', { + name: 'A', + derivation: 'enumerated', + values: [{ description: 'BLUE' }], + }) + + expect(codeOf(store, 'A')).toContain('BLUE') + }) + }) + + // ------------------------------------------------------------------------- + // Data-type code view + // ------------------------------------------------------------------------- + describe('reconcileDatatypeText', () => { + const enumType: PLCDataType = { name: 'Colors', derivation: 'enumerated', values: [{ description: 'RED' }] } + + it('is a no-op when the type is not shown in code mode', () => { + store.getState().projectActions.createDatatype({ data: enumType }) + expect(store.getState().projectActions.reconcileDatatypeText('Colors').ok).toBe(true) + expect(store.getState().project.data.dataTypes[0]).toEqual(enumType) + }) + + it('is a no-op when the buffer still matches the serialized type', () => { + store.getState().projectActions.createDatatype({ data: enumType }) + openDatatypeInCodeMode(store, 'Colors', serializeDataTypeToText(enumType)) + + expect(store.getState().projectActions.reconcileDatatypeText('Colors').ok).toBe(true) + expect(store.getState().project.data.dataTypes[0]).toEqual(enumType) + }) + + it('is a no-op when the type no longer exists', () => { + openDatatypeInCodeMode(store, 'Ghost', 'TYPE\nGhost : (RED);\nEND_TYPE\n') + expect(store.getState().projectActions.reconcileDatatypeText('Ghost').ok).toBe(true) + expect(store.getState().project.data.dataTypes).toHaveLength(0) + }) + + it('folds a diverged buffer back into the type', () => { + store.getState().projectActions.createDatatype({ data: enumType }) + openDatatypeInCodeMode(store, 'Colors', 'TYPE\nColors : (RED, GREEN);\nEND_TYPE\n') + + expect(store.getState().projectActions.reconcileDatatypeText('Colors').ok).toBe(true) + const updated = store.getState().project.data.dataTypes[0] + expect(updated.derivation === 'enumerated' && updated.values).toEqual([ + { description: 'RED' }, + { description: 'GREEN' }, + ]) + }) + + it('refuses when the buffer does not parse', () => { + store.getState().projectActions.createDatatype({ data: enumType }) + openDatatypeInCodeMode(store, 'Colors', 'TYPE\nnot a declaration\nEND_TYPE\n') + + const response = store.getState().projectActions.reconcileDatatypeText('Colors') + expect(response.ok).toBe(false) + expect(response.title).toBe('Data type text is invalid') + expect(store.getState().project.data.dataTypes[0]).toEqual(enumType) + }) + }) + + describe('regenerateDatatypeText', () => { + it('re-serializes the type into its buffer', () => { + const dt: PLCDataType = { name: 'Colors', derivation: 'enumerated', values: [{ description: 'RED' }] } + store.getState().projectActions.createDatatype({ data: dt }) + openDatatypeInCodeMode(store, 'Colors', 'stale text') + + store.getState().projectActions.regenerateDatatypeText('Colors') + expect(codeOf(store, 'Colors')).toBe(serializeDataTypeToText(dt)) + }) + + it('does nothing when the type is not shown in code mode', () => { + const dt: PLCDataType = { name: 'Colors', derivation: 'enumerated', values: [{ description: 'RED' }] } + store.getState().projectActions.createDatatype({ data: dt }) + store.getState().projectActions.regenerateDatatypeText('Colors') + expect(store.getState().editors).toHaveLength(0) + }) + + it('does nothing when the type no longer exists', () => { + openDatatypeInCodeMode(store, 'Ghost', 'raw') + store.getState().projectActions.regenerateDatatypeText('Ghost') + expect(codeOf(store, 'Ghost')).toBe('raw') + }) + }) + + describe('removeUnparsedDataTypeFile', () => { + it('drops only the matching path', () => { + store.getState().projectActions.setUnparsedDataTypeFiles([ + { relativePath: 'datatypes/A.dt', content: 'a' }, + { relativePath: 'datatypes/B.dt', content: 'b' }, + ]) + store.getState().projectActions.removeUnparsedDataTypeFile('datatypes/A.dt') + expect(store.getState().unparsedDataTypeFiles).toEqual([{ relativePath: 'datatypes/B.dt', content: 'b' }]) + }) }) // ========================================================================= diff --git a/src/frontend/store/__tests__/shared-slice.test.ts b/src/frontend/store/__tests__/shared-slice.test.ts index 6e2b688ac..00271ba36 100644 --- a/src/frontend/store/__tests__/shared-slice.test.ts +++ b/src/frontend/store/__tests__/shared-slice.test.ts @@ -577,6 +577,35 @@ describe('createSharedSlice', () => { expect(store.getState().pendingDeletions).toContain('datatypes/OldDT.dt') }) + it('folds pending code-view edits in and rewrites the buffer under the new name', () => { + store.getState().editorActions.updateModelStructureForName('OldDT', { + display: 'code', + code: 'TYPE\nOldDT : STRUCT\nspeed : INT;\nEND_STRUCT;\nEND_TYPE\n', + }) + + expect(store.getState().datatypeActions.rename('OldDT', 'NewDT').ok).toBe(true) + + const renamed = store.getState().project.data.dataTypes[0] + expect(renamed.name).toBe('NewDT') + expect(renamed.derivation === 'structure' && renamed.variable.map((v) => v.name)).toEqual(['speed']) + + const model = store.getState().editor + expect(model.type === 'plc-datatype' && model.structure.display === 'code' && model.structure.code).toContain( + 'NewDT : STRUCT', + ) + }) + + it('refuses the rename while the code view holds invalid text', () => { + store + .getState() + .editorActions.updateModelStructureForName('OldDT', { display: 'code', code: 'TYPE\ngarbage\nEND_TYPE\n' }) + + const result = store.getState().datatypeActions.rename('OldDT', 'NewDT') + expect(result.ok).toBe(false) + expect(store.getState().project.data.dataTypes[0].name).toBe('OldDT') + expect(store.getState().pendingDeletions).not.toContain('datatypes/OldDT.dt') + }) + it('rejects a name owned by an unreadable .dt file (case-insensitive)', () => { store .getState() @@ -1970,6 +1999,33 @@ describe('createSharedSlice', () => { expect(state.files['Configuration']).toBeDefined() }) + it('pre-opens an unreadable .dt file as a code-mode tab without stealing focus', () => { + const data = { + ...makeMinimalProjectResponse(), + unparsedDataTypeFiles: [ + { relativePath: 'datatypes/Broken.dt', content: 'TYPE\nBroken : STRUCT\ngarbage\nEND_TYPE\n' }, + // No name to derive — skipped rather than registered under ''. + { relativePath: '', content: 'orphan' }, + ], + } + store.getState().sharedWorkspaceActions.handleOpenProjectResponse(data) + + const state = store.getState() + expect(state.unparsedDataTypeFiles).toHaveLength(2) + expect(state.files['Broken']).toEqual({ type: 'data-type', filePath: 'Broken', saved: true }) + expect(state.files['']).toBeUndefined() + expect(state.tabs.map((tab) => tab.name)).toEqual(['main', 'Broken']) + // Focus stays on the auto-opened POU. + expect(state.selectedTab).toBe('main') + + const model = state.editors.find((editor) => editor.meta.name === 'Broken') + expect(model?.type === 'plc-datatype' && model.meta.derivation).toBe('structure') + expect(model?.type === 'plc-datatype' && model.structure).toEqual({ + display: 'code', + code: 'TYPE\nBroken : STRUCT\ngarbage\nEND_TYPE\n', + }) + }) + it('logs warnings to console when present', () => { const data = { ...makeMinimalProjectResponse(), diff --git a/src/frontend/store/__tests__/shared-utils.test.ts b/src/frontend/store/__tests__/shared-utils.test.ts index 7a7a8b7de..580daf245 100644 --- a/src/frontend/store/__tests__/shared-utils.test.ts +++ b/src/frontend/store/__tests__/shared-utils.test.ts @@ -6,6 +6,7 @@ import { createEditorObjectForServer, createPouObject, createTabObject, + guessDatatypeDerivation, } from '../slices/shared/utils' describe('shared/utils', () => { @@ -236,7 +237,7 @@ describe('shared/utils', () => { expect(result).toEqual({ type: 'plc-datatype', meta: { name: 'IntArray', derivation: 'array' }, - structure: { description: '', selectedRow: '' }, + structure: { display: 'table', description: '', selectedRow: '-1' }, }) }) @@ -257,6 +258,23 @@ describe('shared/utils', () => { }) }) + // ------------------------------------------------------------------------- + // guessDatatypeDerivation + // ------------------------------------------------------------------------- + describe('guessDatatypeDerivation', () => { + it('detects a structure', () => { + expect(guessDatatypeDerivation('TYPE\nPoint : STRUCT\nx : INT;\nEND_STRUCT;\nEND_TYPE')).toBe('structure') + }) + + it('detects an array', () => { + expect(guessDatatypeDerivation('TYPE\nBuf : ARRAY [0..9] OF INT;\nEND_TYPE')).toBe('array') + }) + + it('falls back to enumerated', () => { + expect(guessDatatypeDerivation('TYPE\nColors : (RED, GREEN);\nEND_TYPE')).toBe('enumerated') + }) + }) + // ------------------------------------------------------------------------- // createEditorObjectForServer // ------------------------------------------------------------------------- diff --git a/src/frontend/store/__tests__/tabs-utils.test.ts b/src/frontend/store/__tests__/tabs-utils.test.ts index 085be585b..926d2a6c8 100644 --- a/src/frontend/store/__tests__/tabs-utils.test.ts +++ b/src/frontend/store/__tests__/tabs-utils.test.ts @@ -94,7 +94,7 @@ describe('tabs/utils', () => { expect(result.type).toBe('plc-datatype') if (result.type === 'plc-datatype') { expect(result.meta.derivation).toBe('enumerated') - expect(result.structure).toEqual({ selectedRow: '-1', description: '' }) + expect(result.structure).toEqual({ display: 'table', selectedRow: '-1', description: '' }) } }) diff --git a/src/frontend/store/slices/editor/slice.ts b/src/frontend/store/slices/editor/slice.ts index 029fb66c5..7b71c7536 100644 --- a/src/frontend/store/slices/editor/slice.ts +++ b/src/frontend/store/slices/editor/slice.ts @@ -1,7 +1,25 @@ import { produce } from 'immer' import { StateCreator } from 'zustand' -import type { EditorSlice, EditorState } from './types' +import type { EditorSlice, EditorState, StructureTableType } from './types' + +const applyStructureView = ( + current: StructureTableType, + data: { display?: 'code' | 'table'; selectedRow?: number; description?: string; code?: string }, +): StructureTableType => { + const display = data.display ?? current.display + if (display === 'table') { + const prevSelectedRow = current.display === 'table' ? current.selectedRow : '-1' + const prevDescription = current.display === 'table' ? current.description : '' + return { + display: 'table', + selectedRow: data.selectedRow !== undefined ? data.selectedRow.toString() : prevSelectedRow, + description: data.description ? data.description : prevDescription, + } + } + const existingCode = current.display === 'code' ? current.code : undefined + return { display: 'code', code: data.code !== undefined ? data.code : existingCode } +} export const createEditorSlice: StateCreator = (setState, getState) => ({ editors: [], @@ -159,19 +177,26 @@ export const createEditorSlice: StateCreator = }), ), - updateModelStructure: ({ selectedRow, description }) => + updateModelStructure: (data) => setState( produce((state: EditorState) => { const { editor } = state if (editor.type === 'plc-datatype') { - editor.structure = { - selectedRow: selectedRow !== undefined ? selectedRow.toString() : editor.structure.selectedRow, - description: description ? description : editor.structure.description, - } + editor.structure = applyStructureView(editor.structure, data) } }), ), + updateModelStructureForName: (name, data) => + setState( + produce((state: EditorState) => { + const targetEditor = + state.editor.meta.name === name ? state.editor : state.editors.find((e) => e.meta.name === name) + if (targetEditor?.type !== 'plc-datatype') return + targetEditor.structure = applyStructureView(targetEditor.structure, data) + }), + ), + updateModelLadder: ({ openRung }) => setState( produce((state: EditorState) => { diff --git a/src/frontend/store/slices/editor/types.ts b/src/frontend/store/slices/editor/types.ts index 42ad52b75..39786825c 100644 --- a/src/frontend/store/slices/editor/types.ts +++ b/src/frontend/store/slices/editor/types.ts @@ -25,10 +25,16 @@ export type GlobalVariablesTableType = code?: string } -export type StructureTableType = { - description: string - selectedRow: string -} +export type StructureTableType = + | { + display: 'table' + description: string + selectedRow: string + } + | { + display: 'code' + code?: string + } export type TaskType = { display: 'table'; selectedRow: string } | { display: 'code' } @@ -253,7 +259,17 @@ export type EditorActions = { code?: string }, ) => void - updateModelStructure: (data: { selectedRow?: number; description?: string }) => void + /** `display` is optional so table-mode row/description updates can't flip the view. */ + updateModelStructure: (data: { + display?: 'code' | 'table' + selectedRow?: number + description?: string + code?: string + }) => void + updateModelStructureForName: ( + name: string, + data: { display?: 'code' | 'table'; selectedRow?: number; description?: string; code?: string }, + ) => void updateModelTasks: (tasks: { selectedRow?: number; display: 'code' | 'table' }) => void updateModelInstances: (instances: { selectedRow?: number; display: 'code' | 'table' }) => void updateModelLadder: (data: { openRung?: { rungId: string; open: boolean } }) => void diff --git a/src/frontend/store/slices/project/slice.ts b/src/frontend/store/slices/project/slice.ts index 1ed7b6605..169b171f4 100644 --- a/src/frontend/store/slices/project/slice.ts +++ b/src/frontend/store/slices/project/slice.ts @@ -36,6 +36,8 @@ import { parseIecStringToVariables } from '../../../utils/generate-iec-string-to import { generateIecVariablesToString } from '../../../utils/generate-iec-variables-to-string' import { isLegalIdentifier } from '../../../utils/keywords' import { DEFAULT_BUFFER_MAPPING } from '../../../utils/modbus/generate-modbus-slave-config' +import { serializeDataTypeToText } from '../../../utils/PLC/data-type-serializer' +import { parseDataTypeFromText } from '../../../utils/PLC/data-type-text-parser' import { getExtensionFromLanguage, getFolderFromPouType } from '../../../utils/PLC/pou-file-extensions' import type { ProjectResponse, ProjectSlice, ProjectSliceRoot } from './types' import { getVariableBasedOnRowIdOrVariableId } from './utils' @@ -504,6 +506,59 @@ const regenerateVariablesText = (pouName: string | undefined, getState: ProjectG state.editorActions.updateModelVariablesForName(pouName, { display: 'code', code: newText }) } +// --------------------------------------------------------------------------- +// Data-type text ⇄ data-type form reconcile helpers +// --------------------------------------------------------------------------- +// +// Same contract as the variables pair above, for the per-type `.dt` +// code view. The form molecules only render in table mode, so the +// reachable divergence cases are a tree rename and an undo/disk +// revert landing while the type sits in code mode with a diverged +// buffer. Rename reconciles first (and refuses when the text is +// invalid, so uncommitted edits are never silently discarded); +// rename and snapshot restore both regenerate afterwards so Monaco +// shows the new state. + +const findDatatypeEditorCode = (name: string, getState: ProjectGetState): string | undefined => { + const state = getState() + const editorModel = state.editor.meta.name === name ? state.editor : state.editors.find((e) => e.meta.name === name) + if (editorModel?.type !== 'plc-datatype') return undefined + if (editorModel.structure.display !== 'code') return undefined + return editorModel.structure.code +} + +const reconcileDatatypeText = (name: string, getState: ProjectGetState, setState: ProjectSetState): ProjectResponse => { + const code = findDatatypeEditorCode(name, getState) + if (typeof code !== 'string') return ok() + + const current = getState().project.data.dataTypes.find((d) => d.name === name) + if (!current) return ok() + // Buffer is a verbatim serialisation of the current type — nothing to fold in. + if (code === serializeDataTypeToText(current)) return ok() + + const { dataType, error } = parseDataTypeFromText(code, name) + if (!dataType) return fail(error ?? 'Unknown parse error.', 'Data type text is invalid') + + setState( + produce((slice: ProjectSlice) => { + const idx = slice.project.data.dataTypes.findIndex((d) => d.name === name) + if (idx !== -1) slice.project.data.dataTypes[idx] = dataType + }), + ) + return ok() +} + +const regenerateDatatypeText = (name: string, getState: ProjectGetState): void => { + if (findDatatypeEditorCode(name, getState) === undefined) return + const state = getState() + const dataType = state.project.data.dataTypes.find((d) => d.name === name) + if (!dataType) return + state.editorActions.updateModelStructureForName(name, { + display: 'code', + code: serializeDataTypeToText(dataType), + }) +} + const createProjectSlice: StateCreator = (setState, getState) => ({ project: { meta: { name: '', type: 'plc-project', path: '' }, @@ -1124,7 +1179,10 @@ const createProjectSlice: StateCreator = if (idx !== -1) slice.project.data.dataTypes[idx] = data }), ) + regenerateDatatypeText(name, getState) }, + reconcileDatatypeText: (name) => reconcileDatatypeText(name, getState, setState), + regenerateDatatypeText: (name) => regenerateDatatypeText(name, getState), setUnparsedDataTypeFiles: (files) => { setState( produce((slice: ProjectSlice) => { @@ -1132,6 +1190,13 @@ const createProjectSlice: StateCreator = }), ) }, + removeUnparsedDataTypeFile: (relativePath) => { + setState( + produce((slice: ProjectSlice) => { + slice.unparsedDataTypeFiles = slice.unparsedDataTypeFiles.filter((f) => f.relativePath !== relativePath) + }), + ) + }, // ----------------------------------------------------------------------- // Tasks diff --git a/src/frontend/store/slices/project/types.ts b/src/frontend/store/slices/project/types.ts index 5a1c1e8c2..015d48e5e 100644 --- a/src/frontend/store/slices/project/types.ts +++ b/src/frontend/store/slices/project/types.ts @@ -223,9 +223,16 @@ export type ProjectActions = { createArrayDimension: (args: { name: string; derivation: 'array' | 'enumerated' | 'structure' }) => void rearrangeStructureVariables: (args: { associatedDataType?: string; rowId: number; newIndex: number }) => void applyDatatypeSnapshot: (name: string, data: PLCDataType) => void + /** Fold a diverged `.dt` code buffer back into the type before an + * external mutation; refuses (`ok: false`) when the text is invalid. */ + reconcileDatatypeText: (name: string) => ProjectResponse + /** Re-serialize the type into its code buffer after an external mutation. */ + regenerateDatatypeText: (name: string) => void /** Stash raw `.dt` files that failed to parse on load so saves echo * them back verbatim (no silent data loss). */ setUnparsedDataTypeFiles: (files: RawProjectFile[]) => void + /** Drop a preserved raw file once its text parses and becomes a real type. */ + removeUnparsedDataTypeFile: (relativePath: string) => void // Tasks createTask: (dto: TaskDTO & { rowToInsert?: number }) => ProjectResponse diff --git a/src/frontend/store/slices/shared/slice.ts b/src/frontend/store/slices/shared/slice.ts index 6922712c9..df6fa89b2 100644 --- a/src/frontend/store/slices/shared/slice.ts +++ b/src/frontend/store/slices/shared/slice.ts @@ -20,7 +20,13 @@ import { } from '../tabs/utils' import { cancelFlowWriteBacks, flushFlowWriteBacks } from './flow-writeback' import type { PouHistorySnapshot, SharedRootState, SharedSlice } from './types' -import { createDatatypeObject, createEditorObjectForDatatype, createEditorObjectForPou, createPouObject } from './utils' +import { + createDatatypeObject, + createEditorObjectForDatatype, + createEditorObjectForPou, + createPouObject, + guessDatatypeDerivation, +} from './utils' const MAX_HISTORY_SIZE = 50 @@ -318,12 +324,22 @@ const createSharedSlice: StateCreator = (s const datatype = state.project.data.dataTypes.find((d) => d.name === oldName) if (!datatype) return { ok: false, message: 'Data type not found' } - return renameElement(state, oldName, newName, () => { + // Fold pending code-view edits in first, so the rename doesn't + // regenerate over them. Invalid text blocks the rename instead + // of silently discarding what the user typed. + const reconcile = state.projectActions.reconcileDatatypeText(oldName) + if (!reconcile.ok) return { ok: false, message: reconcile.message } + + const result = renameElement(state, oldName, newName, () => { // Renames via the dedicated action so the old .dt path gets // queued for deletion — a plain updateDatatype would strand // the old file on disk. state.projectActions.updateDatatypeName(oldName, newName) }) + // After renameElement: both the type and its editor model are + // keyed by newName, so the buffer's TYPE line can be refreshed. + if (result.ok) getState().projectActions.regenerateDatatypeText(newName) + return result }, duplicate: (sourceName, newName) => { @@ -662,6 +678,15 @@ const createSharedSlice: StateCreator = (s // them back verbatim; always set so a reopen clears stale ones. getState().projectActions.setUnparsedDataTypeFiles(data.unparsedDataTypeFiles ?? []) + // No PLCDataType exists for an unreadable file, so it can't show + // up in the project tree. Collected here to get a file entry and a + // pre-opened code-mode tab further down — the only way the user + // can find and fix the declaration without leaving the editor. + const unparsedDataTypes = (data.unparsedDataTypeFiles ?? []).flatMap((file) => { + const name = file.relativePath.split('/').pop()?.replace(/\.dt$/i, '') + return name ? [{ name, content: file.content, derivation: guessDatatypeDerivation(file.content) }] : [] + }) + // Add ladder and FBD flows for graphical POUs. // // The flow object embeds its own `name` field — historically the @@ -870,6 +895,9 @@ const createSharedSlice: StateCreator = (s data.projectData.dataTypes.forEach((dt) => { files[dt.name] = { type: 'data-type', filePath: dt.name, saved: true } }) + unparsedDataTypes.forEach(({ name }) => { + files[name] = { type: 'data-type', filePath: name, saved: true } + }) const servers = data.projectData.servers if (servers) { servers.forEach((s) => { @@ -971,6 +999,20 @@ const createSharedSlice: StateCreator = (s } }) + // Same idea for unreadable .dt files, except the tab has to be + // created too: without a PLCDataType there is no tree leaf to + // click. Focus stays on the auto-opened POU above. + unparsedDataTypes.forEach(({ name, content, derivation }) => { + const tabToBeCreated: TabsProps = { + name, + path: `/data/data-types/${derivation}/${name}`, + elementType: { type: 'data-type', derivation }, + } + getState().tabsActions.updateTabs(tabToBeCreated) + getState().editorActions.addModel(createEditorObjectForDatatype(name, derivation)) + getState().editorActions.updateModelStructureForName(name, { display: 'code', code: content }) + }) + // Reset all graphical flow updated flags at the very end of project open. // Various operations during load (syncNodesWithVariables, debug flag restoration, // tab opening) call updateNode which sets flow.updated = true as a side effect. diff --git a/src/frontend/store/slices/shared/utils.ts b/src/frontend/store/slices/shared/utils.ts index d9e7e1bff..71efb9385 100644 --- a/src/frontend/store/slices/shared/utils.ts +++ b/src/frontend/store/slices/shared/utils.ts @@ -141,10 +141,20 @@ export function createEditorObjectForDatatype(name: string, derivation: string): return { type: 'plc-datatype', meta: { name, derivation: derivation as 'enumerated' | 'structure' | 'array' }, - structure: { description: '', selectedRow: '' }, + structure: { display: 'table', description: '', selectedRow: '-1' }, } } +/** + * Best-effort derivation for a `.dt` file that failed to parse — it only + * picks the tab/tree icon, since no `PLCDataType` exists to read it from. + */ +export function guessDatatypeDerivation(content: string): 'enumerated' | 'structure' | 'array' { + if (/\bSTRUCT\b/i.test(content)) return 'structure' + if (/\bARRAY\b/i.test(content)) return 'array' + return 'enumerated' +} + export function createEditorObjectForServer( name: string, protocol: 'modbus-tcp' | 's7comm' | 'ethernet-ip' | 'opcua', diff --git a/src/frontend/store/slices/tabs/utils.ts b/src/frontend/store/slices/tabs/utils.ts index bb04f5098..f3e13db48 100644 --- a/src/frontend/store/slices/tabs/utils.ts +++ b/src/frontend/store/slices/tabs/utils.ts @@ -57,7 +57,7 @@ const CreateEditorModelObject = ( return { type: 'plc-datatype', meta: { name, derivation }, - structure: { selectedRow: '-1', description: '' }, + structure: { display: 'table', selectedRow: '-1', description: '' }, } } From 8c419a1dcdb868805b08c1c686dabf6da8d28b0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20GS=20Pereira?= Date: Thu, 6 Aug 2026 16:58:35 -0300 Subject: [PATCH 2/2] fix(data-types): make a failed code commit fire once, and keep unreadable .dt files off taken names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking away from the code view raises document `mousedown` and then container `focusout`. The commit is synchronous, so `isParsingRef` is already clear by the second event, and a failure left `lastParsedCodeRef` un-advanced — so the same invalid buffer was parsed twice and toasted twice. Track the rejected buffer as well, so the pair is one attempt whatever its outcome. The variables editor never hit this because its commit is async and its latch spans both events. An unreadable `datatypes/.dt` took its name from the file basename with nothing checking it against the elements already loaded. POUs and data types share one identifier namespace and the file registry is keyed by raw name, so a project holding both a POU `foo` and an unreadable `foo.dt` had that POU's registry entry retyped to `data-type` — enough to send its next save down the `.dt` branch. Skip colliding names; the file still rides along in `unparsedDataTypeFiles`, so it is echoed back to disk rather than dropped. Comments across the feature trimmed to the non-obvious constraints. Refs DOPE-535 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GvGrhB1LyJz9MBwYhjoBDY --- .../_features/[workspace]/data-type/index.tsx | 37 ++++++++++--------- .../store/__tests__/shared-slice.test.ts | 26 +++++++++++++ src/frontend/store/slices/project/slice.ts | 13 +------ src/frontend/store/slices/shared/slice.ts | 24 ++++++------ src/frontend/store/slices/shared/utils.ts | 5 +-- 5 files changed, 60 insertions(+), 45 deletions(-) diff --git a/src/frontend/components/_features/[workspace]/data-type/index.tsx b/src/frontend/components/_features/[workspace]/data-type/index.tsx index fc46a36c0..44190d68a 100644 --- a/src/frontend/components/_features/[workspace]/data-type/index.tsx +++ b/src/frontend/components/_features/[workspace]/data-type/index.tsx @@ -40,17 +40,15 @@ const DataTypeEditor = ({ dataTypeName, ...rest }: DatatypeEditorProps) => { } = useOpenPLCStore() const { captureAndPush } = usePouSnapshot() - // Every open data type is mounted at once (workspace-screen keeps the - // inactive ones hidden), so the view state has to come from this - // type's own model — never from the active `editor`. + // Every open data type is mounted at once, so the view state comes from + // this type's own model — never from the active `editor`. const model = editor.meta.name === dataTypeName ? editor : editors.find((e) => e.meta.name === dataTypeName) const modelStructure = model?.type === 'plc-datatype' ? model.structure : undefined const codeViewEnabled = isDataTypeFilesEnabled() const display = codeViewEnabled && modelStructure?.display === 'code' ? 'code' : 'table' const modelCode = modelStructure?.display === 'code' ? modelStructure.code : undefined - // A `.dt` file that failed to parse has no entry in `dataTypes`; the - // raw text is all there is until the user fixes it. + // An unparseable file has no entry in `dataTypes` — raw text is all there is. const rawFile = unparsedDataTypeFiles.find( (file) => file.relativePath.split('/').pop()?.replace(/\.dt$/i, '') === dataTypeName, ) @@ -68,6 +66,7 @@ const DataTypeEditor = ({ dataTypeName, ...rest }: DatatypeEditorProps) => { const latestCodeRef = useRef(editorCode) const latestDisplayRef = useRef(display) const lastParsedCodeRef = useRef(editorCode) + const lastRejectedCodeRef = useRef(null) const lastMirroredCodeRef = useRef(editorCode) const isParsingRef = useRef(false) const commitCodeRef = useRef<() => boolean>(() => false) @@ -77,20 +76,17 @@ const DataTypeEditor = ({ dataTypeName, ...rest }: DatatypeEditorProps) => { if (dataType) setEditorContent(dataType) }, [dataTypes, dataTypeName]) - // Keep the buffer serialized from the form while in table mode, so the - // toggle already holds the right text the moment it flips. + // In table mode the form is the committed state: it seeds both the buffer + // and the watermark, so the toggle is instant and can't commit a no-op. useEffect(() => { if (display === 'code') return const text = editorContent ? serializeDataTypeToText(editorContent) : (rawFile?.content ?? '') setEditorCode(text) - // In table mode the form is the committed state, so this is also - // the watermark the next outside-click compares against. lastParsedCodeRef.current = text }, [editorContent, display, rawFile?.content]) - // Adopt buffers written by the store (a tree rename or an undo - // regenerates them), but never the echo of our own mirror below — - // that would race the keystroke that produced it. + // Adopt store-written buffers (rename, undo), never the echo of our own + // mirror below — that would race the keystroke that produced it. useEffect(() => { if (display !== 'code' || typeof modelCode !== 'string') return if (modelCode === lastMirroredCodeRef.current) return @@ -116,8 +112,7 @@ const DataTypeEditor = ({ dataTypeName, ...rest }: DatatypeEditorProps) => { } }, [dataTypeName, updateModelStructureForName]) - // A type that doesn't exist yet is a broken file on disk: show why it - // is broken while the user edits, instead of only on commit. + // No type yet means a broken file — surface why while editing, not on commit. useEffect(() => { if (display !== 'code' || editorContent) return setParseError(parseDataTypeFromText(editorCode, dataTypeName).error ?? null) @@ -159,11 +154,20 @@ const DataTypeEditor = ({ dataTypeName, ...rest }: DatatypeEditorProps) => { useEffect(() => { if (display !== 'code') return + // Clicking away raises mousedown then focusout, and the commit is + // synchronous, so `isParsingRef` is clear by the second one. Both + // watermarks make the pair one attempt whatever its outcome. const tryCommit = () => { if (isParsingRef.current) return if (editorCode === lastParsedCodeRef.current) return + if (editorCode === lastRejectedCodeRef.current) return isParsingRef.current = true - if (commitCodeRef.current()) lastParsedCodeRef.current = editorCode + if (commitCodeRef.current()) { + lastParsedCodeRef.current = editorCode + lastRejectedCodeRef.current = null + } else { + lastRejectedCodeRef.current = editorCode + } isParsingRef.current = false } @@ -173,8 +177,7 @@ const DataTypeEditor = ({ dataTypeName, ...rest }: DatatypeEditorProps) => { tryCommit() } - // Covers keyboard navigation, Tab and shortcuts — anything that - // moves focus away without a mousedown. + // Covers focus moves with no mousedown: Tab, shortcuts. const onFocusOut = (e: FocusEvent) => { if (!containerRef.current) return const newTarget = e.relatedTarget as Node | null diff --git a/src/frontend/store/__tests__/shared-slice.test.ts b/src/frontend/store/__tests__/shared-slice.test.ts index 00271ba36..066e640e9 100644 --- a/src/frontend/store/__tests__/shared-slice.test.ts +++ b/src/frontend/store/__tests__/shared-slice.test.ts @@ -2026,6 +2026,32 @@ describe('createSharedSlice', () => { }) }) + it('does not let an unreadable .dt displace a POU or a parsed type of the same name', () => { + const data = makeMinimalProjectResponse() + data.projectData.dataTypes = [ + { name: 'Colors', derivation: 'enumerated', values: [{ description: 'RED' }], initialValue: '' }, + ] as typeof data.projectData.dataTypes + const withCollisions = { + ...data, + unparsedDataTypeFiles: [ + // Case-insensitive: the filesystem folds case, the registry doesn't. + { relativePath: 'datatypes/MAIN.dt', content: 'TYPE\ngarbage\nEND_TYPE\n' }, + { relativePath: 'datatypes/colors.dt', content: 'TYPE\ngarbage\nEND_TYPE\n' }, + ], + } + store.getState().sharedWorkspaceActions.handleOpenProjectResponse(withCollisions) + + const state = store.getState() + // The POU keeps its own registry entry, tab and model. + expect(state.files['main'].type).toBe('program') + expect(state.tabs.map((tab) => tab.name)).toEqual(['main']) + expect(state.editors.every((editor) => editor.type !== 'plc-datatype')).toBe(true) + expect(state.files['MAIN']).toBeUndefined() + expect(state.files['colors']).toBeUndefined() + // Still preserved, so the next save echoes both files back verbatim. + expect(state.unparsedDataTypeFiles).toHaveLength(2) + }) + it('logs warnings to console when present', () => { const data = { ...makeMinimalProjectResponse(), diff --git a/src/frontend/store/slices/project/slice.ts b/src/frontend/store/slices/project/slice.ts index 169b171f4..21a00faf0 100644 --- a/src/frontend/store/slices/project/slice.ts +++ b/src/frontend/store/slices/project/slice.ts @@ -506,18 +506,7 @@ const regenerateVariablesText = (pouName: string | undefined, getState: ProjectG state.editorActions.updateModelVariablesForName(pouName, { display: 'code', code: newText }) } -// --------------------------------------------------------------------------- -// Data-type text ⇄ data-type form reconcile helpers -// --------------------------------------------------------------------------- -// -// Same contract as the variables pair above, for the per-type `.dt` -// code view. The form molecules only render in table mode, so the -// reachable divergence cases are a tree rename and an undo/disk -// revert landing while the type sits in code mode with a diverged -// buffer. Rename reconciles first (and refuses when the text is -// invalid, so uncommitted edits are never silently discarded); -// rename and snapshot restore both regenerate afterwards so Monaco -// shows the new state. +// Same contract as the variables pair above, for the `.dt` code view. const findDatatypeEditorCode = (name: string, getState: ProjectGetState): string | undefined => { const state = getState() diff --git a/src/frontend/store/slices/shared/slice.ts b/src/frontend/store/slices/shared/slice.ts index df6fa89b2..1435e28ab 100644 --- a/src/frontend/store/slices/shared/slice.ts +++ b/src/frontend/store/slices/shared/slice.ts @@ -325,8 +325,7 @@ const createSharedSlice: StateCreator = (s if (!datatype) return { ok: false, message: 'Data type not found' } // Fold pending code-view edits in first, so the rename doesn't - // regenerate over them. Invalid text blocks the rename instead - // of silently discarding what the user typed. + // regenerate over them. const reconcile = state.projectActions.reconcileDatatypeText(oldName) if (!reconcile.ok) return { ok: false, message: reconcile.message } @@ -336,8 +335,7 @@ const createSharedSlice: StateCreator = (s // the old file on disk. state.projectActions.updateDatatypeName(oldName, newName) }) - // After renameElement: both the type and its editor model are - // keyed by newName, so the buffer's TYPE line can be refreshed. + // Only after renameElement are the type and its model both keyed by newName. if (result.ok) getState().projectActions.regenerateDatatypeText(newName) return result }, @@ -678,13 +676,17 @@ const createSharedSlice: StateCreator = (s // them back verbatim; always set so a reopen clears stale ones. getState().projectActions.setUnparsedDataTypeFiles(data.unparsedDataTypeFiles ?? []) - // No PLCDataType exists for an unreadable file, so it can't show - // up in the project tree. Collected here to get a file entry and a - // pre-opened code-mode tab further down — the only way the user - // can find and fix the declaration without leaving the editor. + // Unreadable files have no PLCDataType, so no tree leaf to click. const unparsedDataTypes = (data.unparsedDataTypeFiles ?? []).flatMap((file) => { const name = file.relativePath.split('/').pop()?.replace(/\.dt$/i, '') - return name ? [{ name, content: file.content, derivation: guessDatatypeDerivation(file.content) }] : [] + if (!name) return [] + // The file registry is keyed by raw name across both kinds: a + // colliding file would retype the real element and misroute its save. + const taken = [...data.projectData.pous, ...data.projectData.dataTypes].some( + (element) => element.name.toLowerCase() === name.toLowerCase(), + ) + if (taken) return [] + return [{ name, content: file.content, derivation: guessDatatypeDerivation(file.content) }] }) // Add ladder and FBD flows for graphical POUs. @@ -999,9 +1001,7 @@ const createSharedSlice: StateCreator = (s } }) - // Same idea for unreadable .dt files, except the tab has to be - // created too: without a PLCDataType there is no tree leaf to - // click. Focus stays on the auto-opened POU above. + // Tab included, and focus stays on the auto-opened POU above. unparsedDataTypes.forEach(({ name, content, derivation }) => { const tabToBeCreated: TabsProps = { name, diff --git a/src/frontend/store/slices/shared/utils.ts b/src/frontend/store/slices/shared/utils.ts index 71efb9385..5972bbb91 100644 --- a/src/frontend/store/slices/shared/utils.ts +++ b/src/frontend/store/slices/shared/utils.ts @@ -145,10 +145,7 @@ export function createEditorObjectForDatatype(name: string, derivation: string): } } -/** - * Best-effort derivation for a `.dt` file that failed to parse — it only - * picks the tab/tree icon, since no `PLCDataType` exists to read it from. - */ +/** Best-effort derivation for an unparseable `.dt` — only picks the icon. */ export function guessDatatypeDerivation(content: string): 'enumerated' | 'structure' | 'array' { if (/\bSTRUCT\b/i.test(content)) return 'structure' if (/\bARRAY\b/i.test(content)) return 'array'