Skip to content

Commit 5fec6d9

Browse files
KiteniteKitenite
authored
fix: handle multiple oids and invalid oids (#2906)
1 parent 1459ed8 commit 5fec6d9

File tree

2 files changed

+1165
-72
lines changed

2 files changed

+1165
-72
lines changed

‎packages/parser/src/ids.ts‎

Lines changed: 207 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,129 @@
11
import { EditorAttributes } from '@onlook/constants';
22
import { createOid } from '@onlook/utility';
33

4-
import type { NodePath, T } from './packages';
54
import { isReactFragment } from './helpers';
5+
import type { T } from './packages';
66
import { t, traverse } from './packages';
77

8+
/**
9+
* Generates a unique OID that doesn't conflict with global or local OIDs
10+
*/
11+
function generateUniqueOid(globalOids: Set<string>, localOids: Set<string>): string {
12+
let newOid: string;
13+
do {
14+
newOid = createOid();
15+
} while (globalOids.has(newOid) || localOids.has(newOid));
16+
return newOid;
17+
}
18+
19+
/**
20+
* Creates a new JSX data-oid attribute with the given value
21+
*/
22+
function createOidAttribute(oidValue: string): T.JSXAttribute {
23+
return t.jSXAttribute(
24+
t.jSXIdentifier(EditorAttributes.DATA_ONLOOK_ID),
25+
t.stringLiteral(oidValue),
26+
);
27+
}
28+
29+
/**
30+
* Removes all existing OID attributes from the element
31+
*/
32+
function removeAllOidAttributes(
33+
attributes: (T.JSXAttribute | T.JSXSpreadAttribute)[],
34+
indices: number[],
35+
): void {
36+
// Remove in reverse order to maintain correct indices
37+
indices
38+
.sort((a, b) => b - a)
39+
.forEach((index) => {
40+
attributes.splice(index, 1);
41+
});
42+
}
43+
44+
/**
45+
* Checks if an OID should be replaced due to branch or local conflicts
46+
*/
47+
function shouldReplaceOid(
48+
oidValue: string,
49+
globalOids: Set<string>,
50+
localOids: Set<string>,
51+
branchOidMap: Map<string, string>,
52+
currentBranchId?: string,
53+
): boolean {
54+
const oidOwnerBranch = branchOidMap.get(oidValue);
55+
56+
// Replace OID if:
57+
// 1. It exists globally AND belongs to a different branch, OR
58+
// 2. It's already used elsewhere in this same AST
59+
return (
60+
(globalOids.has(oidValue) && oidOwnerBranch && oidOwnerBranch !== currentBranchId) ||
61+
localOids.has(oidValue)
62+
);
63+
}
64+
65+
/**
66+
* Handles elements that have multiple or invalid OIDs by removing all and creating a new one
67+
*/
68+
function handleProblematicOids(
69+
attributes: (T.JSXAttribute | T.JSXSpreadAttribute)[],
70+
oidIndices: number[],
71+
globalOids: Set<string>,
72+
localOids: Set<string>,
73+
): string {
74+
// Remove all existing OID attributes
75+
removeAllOidAttributes(attributes, oidIndices);
76+
77+
// Generate and add new unique OID
78+
const newOid = generateUniqueOid(globalOids, localOids);
79+
const newOidAttribute = createOidAttribute(newOid);
80+
attributes.push(newOidAttribute);
81+
localOids.add(newOid);
82+
83+
return newOid;
84+
}
85+
86+
/**
87+
* Handles elements with a single valid OID by checking for conflicts and replacing if necessary
88+
*/
89+
function handleSingleValidOid(
90+
attributes: (T.JSXAttribute | T.JSXSpreadAttribute)[],
91+
oidValue: string,
92+
oidIndex: number,
93+
globalOids: Set<string>,
94+
localOids: Set<string>,
95+
branchOidMap: Map<string, string>,
96+
currentBranchId?: string,
97+
): { oidValue: string; wasReplaced: boolean } {
98+
if (shouldReplaceOid(oidValue, globalOids, localOids, branchOidMap, currentBranchId)) {
99+
// Generate new unique OID and replace the existing one
100+
const newOid = generateUniqueOid(globalOids, localOids);
101+
const attr = attributes[oidIndex] as T.JSXAttribute;
102+
attr.value = t.stringLiteral(newOid);
103+
localOids.add(newOid);
104+
return { oidValue: newOid, wasReplaced: true };
105+
} else {
106+
// Keep existing OID and track it locally
107+
localOids.add(oidValue);
108+
return { oidValue, wasReplaced: false };
109+
}
110+
}
111+
112+
/**
113+
* Handles elements with no OID by creating a new one
114+
*/
115+
function handleMissingOid(
116+
attributes: (T.JSXAttribute | T.JSXSpreadAttribute)[],
117+
globalOids: Set<string>,
118+
localOids: Set<string>,
119+
): string {
120+
const newOid = generateUniqueOid(globalOids, localOids);
121+
const newOidAttribute = createOidAttribute(newOid);
122+
attributes.push(newOidAttribute);
123+
localOids.add(newOid);
124+
return newOid;
125+
}
126+
8127
export function addOidsToAst(
9128
ast: T.File,
10129
globalOids = new Set<string>(),
@@ -14,65 +133,94 @@ export function addOidsToAst(
14133
let modified = false;
15134
// Track OIDs used within this AST to prevent duplicates in the same file
16135
const localOids = new Set<string>();
136+
17137
traverse(ast, {
18138
JSXOpeningElement(path) {
19139
if (isReactFragment(path.node)) {
20140
return;
21141
}
142+
22143
const attributes = path.node.attributes;
23-
const existingOid = getExistingOid(attributes);
24-
25-
if (existingOid) {
26-
// If the element already has an oid, check if it conflicts with other branches or local duplicates
27-
const { value, index } = existingOid;
28-
const oidOwnerBranch = branchOidMap.get(value);
29-
30-
// Replace OID if:
31-
// 1. It exists globally AND belongs to a different branch, OR
32-
// 2. It's already used elsewhere in this same AST
33-
if (
34-
(globalOids.has(value) &&
35-
oidOwnerBranch &&
36-
oidOwnerBranch !== currentBranchId) ||
37-
localOids.has(value)
38-
) {
39-
// Generate a new unique OID that doesn't conflict globally or locally
40-
let newOid: string;
41-
do {
42-
newOid = createOid();
43-
} while (globalOids.has(newOid) || localOids.has(newOid));
44-
45-
const attr = attributes[index] as T.JSXAttribute;
46-
attr.value = t.stringLiteral(newOid);
47-
localOids.add(newOid);
144+
const allOids = getAllExistingOids(attributes);
145+
146+
if (allOids.indices.length > 0) {
147+
if (allOids.hasMultiple || allOids.hasInvalid) {
148+
// Handle multiple or invalid OIDs: remove all and create new one
149+
handleProblematicOids(attributes, allOids.indices, globalOids, localOids);
48150
modified = true;
49151
} else {
50-
// Keep existing OID and track it locally for future duplicate detection
51-
localOids.add(value);
152+
// Handle single valid OID: check for conflicts
153+
const oidValue = allOids.values[0];
154+
const oidIndex = allOids.indices[0];
155+
156+
if (oidValue && oidIndex !== undefined) {
157+
const result = handleSingleValidOid(
158+
attributes,
159+
oidValue,
160+
oidIndex,
161+
globalOids,
162+
localOids,
163+
branchOidMap,
164+
currentBranchId,
165+
);
166+
if (result.wasReplaced) {
167+
modified = true;
168+
}
169+
}
52170
}
53171
} else {
54-
// If the element doesn't have an oid, create one
55-
let newOid: string;
56-
do {
57-
newOid = createOid();
58-
} while (globalOids.has(newOid) || localOids.has(newOid));
59-
60-
const newOidAttribute = t.jSXAttribute(
61-
t.jSXIdentifier(EditorAttributes.DATA_ONLOOK_ID),
62-
t.stringLiteral(newOid),
63-
);
64-
attributes.push(newOidAttribute);
65-
localOids.add(newOid);
172+
// Handle missing OID: create new one
173+
handleMissingOid(attributes, globalOids, localOids);
66174
modified = true;
67175
}
68176
},
69177
});
178+
70179
return { ast, modified };
71180
}
72181

182+
export function getAllExistingOids(attributes: (T.JSXAttribute | T.JSXSpreadAttribute)[]): {
183+
indices: number[];
184+
values: string[];
185+
hasMultiple: boolean;
186+
hasInvalid: boolean;
187+
} {
188+
const oidIndices: number[] = [];
189+
const oidValues: string[] = [];
190+
let hasInvalid = false;
191+
192+
attributes.forEach((attr, index) => {
193+
if (t.isJSXAttribute(attr) && attr.name.name === EditorAttributes.DATA_ONLOOK_ID) {
194+
oidIndices.push(index);
195+
196+
const existingAttrValue = attr.value;
197+
if (!existingAttrValue || !t.isStringLiteral(existingAttrValue)) {
198+
hasInvalid = true;
199+
oidValues.push('');
200+
} else {
201+
const value = existingAttrValue.value;
202+
// Treat empty strings and whitespace-only strings as invalid
203+
if (!value || value.trim() === '') {
204+
hasInvalid = true;
205+
oidValues.push('');
206+
} else {
207+
oidValues.push(value);
208+
}
209+
}
210+
}
211+
});
212+
213+
return {
214+
indices: oidIndices,
215+
values: oidValues,
216+
hasMultiple: oidIndices.length > 1,
217+
hasInvalid,
218+
};
219+
}
220+
73221
export function getExistingOid(
74222
attributes: (T.JSXAttribute | T.JSXSpreadAttribute)[],
75-
): { value: string; index: number } | null {
223+
): { value: string; index: number; shouldRemove: boolean } | null {
76224
const existingAttrIndex = attributes.findIndex(
77225
(attr) => t.isJSXAttribute(attr) && attr.name.name === EditorAttributes.DATA_ONLOOK_ID,
78226
);
@@ -93,40 +241,27 @@ export function getExistingOid(
93241

94242
const existingAttrValue = existingAttr.value;
95243
if (!existingAttrValue || !t.isStringLiteral(existingAttrValue)) {
96-
return null;
244+
// Mark invalid oid attributes for removal
245+
return {
246+
index: existingAttrIndex,
247+
value: '',
248+
shouldRemove: true,
249+
};
250+
}
251+
252+
const value = existingAttrValue.value;
253+
// Treat empty strings and whitespace-only strings as invalid
254+
if (!value || value.trim() === '') {
255+
return {
256+
index: existingAttrIndex,
257+
value: '',
258+
shouldRemove: true,
259+
};
97260
}
98261

99262
return {
100263
index: existingAttrIndex,
101-
value: existingAttrValue.value,
264+
value,
265+
shouldRemove: false,
102266
};
103267
}
104-
105-
export function removeOidsFromAst(ast: T.File) {
106-
traverse(ast, {
107-
JSXOpeningElement(path: NodePath<T.JSXOpeningElement>) {
108-
if (isReactFragment(path.node)) {
109-
return;
110-
}
111-
const attributes = path.node.attributes;
112-
const existingAttrIndex = attributes.findIndex(
113-
(attr: any) => attr.name?.name === EditorAttributes.DATA_ONLOOK_ID,
114-
);
115-
116-
if (existingAttrIndex !== -1) {
117-
attributes.splice(existingAttrIndex, 1);
118-
}
119-
},
120-
JSXAttribute(path: NodePath<T.JSXAttribute>) {
121-
if (path.node.name.name === 'key') {
122-
const value = path.node.value;
123-
if (
124-
t.isStringLiteral(value) &&
125-
value.value.startsWith(EditorAttributes.ONLOOK_MOVE_KEY_PREFIX)
126-
) {
127-
return path.remove();
128-
}
129-
}
130-
},
131-
});
132-
}

0 commit comments

Comments
 (0)