Closed pastelmind closed 1 year ago
Preventing the addition of new properties should be trivial. We can call Object.preventExtensions()
on the snapshot object here, just before returning it here:
diff --git a/src/vanilla.ts b/src/vanilla.ts
index 7844994..1e169a0 100644
--- a/src/vanilla.ts
+++ b/src/vanilla.ts
@@ -146,6 +146,7 @@ const buildProxyFunction = (
}
Object.defineProperty(snap, key, desc)
})
+ Object.preventExtensions(snap)
return snap
},
I tested this locally and found no breaking tests. I can submit a PR if you want.
However, preventing deletion would be a bit more involved. It's hard to grok the details, but I believe snapshot properties were made configurable in ba53bf6fec891ec82a6fca211c562e49dbc38aae (#664) due to performance issues. Would it be too difficult to revert some of the changes?
If we want to avoid reverting the above, I suppose we might be able to wrap the snapshot object in its own Proxy
with a deleteProperty
handler that throws a TypeError
. This sounds too complicated and risky, though.
We used to deep-freeze snapshot object. But, there's performance downside, because we had to deep-copy the object in proxy-compare
. So, it means we took performance over correctness.
Object.preventExtensions()
sounds good. If it doesn't break existing tests and it doesn't drop performance (but, we don't have tests for it), we can merge it.
Summary
Adding or removing properties from a snapshot object or array does not throw, even in strict mode.
Although it appears that such invalid operations do not affect the proxy store itself,
silently swallowingallowing these errors is undesirable, since the snapshot (correctly) errors on other types of mutations.Edit: The snapshot can be mutated:
This seems to be a correctness issue.
Link to reproduction
https://codesandbox.io/s/sad-gauss-w364gf
Check List
Please do not ask questions in issues.
Please include a minimal reproduction.