mirror of
https://github.com/webstudio-is/webstudio.git
synced 2026-10-23 03:27:27 +09:00
fix: Prevent Editor props from shadowing bindings
This commit is contained in:
1 parent
4591940169
commit
5dd03135e7
4 files changed
+156
-10
No files matched your search
@@ -452,6 +452,121 @@ test("does not reuse an editable prop id for a different name", () => {
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
test.each(["expression", "string"] as const)(
|
||||
"rejects a new prop ID shadowing an existing %s prop",
|
||||
(type) => {
|
||||
const existing: Prop = {
|
||||
id: "href-prop",
|
||||
instanceId: "link",
|
||||
name: "href",
|
||||
type,
|
||||
value: type === "expression" ? "post.url" : "/original",
|
||||
};
|
||||
const capabilities = getContentModeCapabilities({
|
||||
instances,
|
||||
props: new Map([[existing.id, existing]]),
|
||||
metas,
|
||||
styleSources,
|
||||
});
|
||||
const original = structuredClone(capabilities);
|
||||
for (const split of [false, true]) {
|
||||
const patches: Patch[] = [
|
||||
{
|
||||
op: "add",
|
||||
path: ["shadow"],
|
||||
value: {
|
||||
...existing,
|
||||
id: "shadow",
|
||||
type: "string",
|
||||
value: "/override",
|
||||
},
|
||||
},
|
||||
];
|
||||
if (split) {
|
||||
patches.push({
|
||||
op: "replace",
|
||||
path: ["shadow", "value"],
|
||||
value: "/again",
|
||||
});
|
||||
}
|
||||
expect(
|
||||
applyContentModeTransaction({
|
||||
capabilities,
|
||||
scope: "text-and-props",
|
||||
transaction: {
|
||||
payload: patches.map((patch) => ({
|
||||
namespace: "props",
|
||||
patches: [patch],
|
||||
})),
|
||||
},
|
||||
}).success
|
||||
).toBe(false);
|
||||
}
|
||||
expect(
|
||||
applyContentModeTransaction({
|
||||
capabilities,
|
||||
scope: "text-and-props",
|
||||
transaction: transaction("props", [
|
||||
{
|
||||
op: "replace",
|
||||
path: [existing.id],
|
||||
value: { ...existing, type: "string", value: "/updated" },
|
||||
},
|
||||
]),
|
||||
}).success
|
||||
).toBe(type === "string");
|
||||
expect(capabilities).toEqual(original);
|
||||
}
|
||||
);
|
||||
|
||||
test("rejects fresh linked Image dimensions shadowing a binding", () => {
|
||||
const image: Instance = {
|
||||
id: "image",
|
||||
type: "instance",
|
||||
component: "Image",
|
||||
children: [],
|
||||
};
|
||||
const source: Prop = {
|
||||
id: "src",
|
||||
instanceId: image.id,
|
||||
name: "src",
|
||||
type: "asset",
|
||||
value: "asset",
|
||||
};
|
||||
const width: Prop = {
|
||||
id: "width",
|
||||
instanceId: image.id,
|
||||
name: "width",
|
||||
type: "expression",
|
||||
value: "post.width",
|
||||
};
|
||||
const capabilities = getContentModeCapabilities({
|
||||
instances: new Map([[image.id, image]]),
|
||||
contentRootIds: new Set([image.id]),
|
||||
props: new Map([
|
||||
[source.id, source],
|
||||
[width.id, width],
|
||||
]),
|
||||
metas: new Map(),
|
||||
styleSources,
|
||||
});
|
||||
expect(
|
||||
applyContentModeTransaction({
|
||||
capabilities,
|
||||
scope: "text-and-props",
|
||||
transaction: transaction("props", [
|
||||
{
|
||||
op: "add",
|
||||
path: ["shadow"],
|
||||
value: { ...width, id: "shadow", type: "asset", value: source.value },
|
||||
},
|
||||
]),
|
||||
}).success
|
||||
).toBe(false);
|
||||
expect(capabilities.props.get(width.id)).toEqual(width);
|
||||
expect(capabilities.props.has("shadow")).toBe(false);
|
||||
});
|
||||
|
||||
const metas = new Map<string, WsComponentMeta>([
|
||||
[
|
||||
"Link",
|
||||
@@ -1193,9 +1308,9 @@ describe("content mode permissions", () => {
|
||||
isContentModePropTransition({
|
||||
capabilities,
|
||||
editableInstanceIds: new Set(["link"]),
|
||||
expectedPropId: "new-href-prop",
|
||||
expectedPropId: "href-prop",
|
||||
value: {
|
||||
id: "new-href-prop",
|
||||
id: "href-prop",
|
||||
instanceId: "link",
|
||||
name: "href",
|
||||
type: "string",
|
||||
|
||||
@@ -395,6 +395,17 @@ const isContentModePropTransition = ({
|
||||
}
|
||||
|
||||
const existingProp = capabilities.props.get(expectedPropId);
|
||||
if (
|
||||
existingProp === undefined &&
|
||||
Array.from(capabilities.props.values()).some(
|
||||
(prop) =>
|
||||
prop.instanceId === instance.id &&
|
||||
prop.name === parsedProp.data.name &&
|
||||
nextProps.has(prop.id)
|
||||
)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
if (existingProp !== undefined) {
|
||||
if (
|
||||
existingProp.instanceId !== instance.id ||
|
||||
|
||||
@@ -511,6 +511,27 @@ describe("createPropUpsertPayload", () => {
|
||||
});
|
||||
|
||||
describe("updateProps", () => {
|
||||
test("coalescing reads a large rejected batch only linearly", () => {
|
||||
let reads = 0;
|
||||
const updates = Array.from({ length: 200 }, (_, index) => ({
|
||||
get instanceId() {
|
||||
reads += 1;
|
||||
return "missing";
|
||||
},
|
||||
name: `prop-${index}`,
|
||||
type: "string" as const,
|
||||
value: "value",
|
||||
}));
|
||||
expect(() =>
|
||||
updateProps(
|
||||
{ instances: new Map([[image.id, image]]), props: new Map() },
|
||||
{ updates },
|
||||
{ createId: () => "unused" }
|
||||
)
|
||||
).toThrow();
|
||||
expect(reads).toBeLessThanOrEqual(updates.length * 3);
|
||||
});
|
||||
|
||||
test("coalesces by instance and property without merging unrelated updates", () => {
|
||||
const other = { ...image, id: "other", component: "Link" };
|
||||
const existing = prop("title", { value: "Original" });
|
||||
|
||||
@@ -928,14 +928,13 @@ export const updateProps = (
|
||||
context: { createId: () => string }
|
||||
) => {
|
||||
const { instances, props } = getRequiredPropState(state);
|
||||
const updates = input.updates.filter(
|
||||
(update, index, updates) =>
|
||||
updates.findLastIndex(
|
||||
(candidate) =>
|
||||
candidate.instanceId === update.instanceId &&
|
||||
candidate.name === update.name
|
||||
) === index
|
||||
);
|
||||
const updatesByProp = new Map<string, (typeof input.updates)[number]>();
|
||||
for (const update of input.updates) {
|
||||
const key = JSON.stringify([update.instanceId, update.name]);
|
||||
updatesByProp.delete(key);
|
||||
updatesByProp.set(key, update);
|
||||
}
|
||||
const updates = [...updatesByProp.values()];
|
||||
const nextProps = updates.map((update) => {
|
||||
assertRuntimeInstance(instances, update.instanceId);
|
||||
const instance = instances.get(update.instanceId)!;
|
||||
|
||||
Reference in new issue
Block a user