fix reactive objects which are sealed

[neelance]

What kind of change does this PR introduce? (check at least one)

• Bugfix
• Feature
• Code style update
• Refactor
• Build-related changes
• Other, please describe:

Does this PR introduce a breaking change? (check one)

• Yes
• No

I am using this helper method to seal my objects before passing them to Vue:

function makeReactiveAndSeal(obj) {
  if (!obj.__ob__) {
    Object.keys(obj).forEach((key) => {
      Vue.util.defineReactive(obj, key, obj[key]);
    });
  }
  Object.seal(obj);
}

This turns silent errors into nice verbose errors, for example when there is a mistake in a Vue template. The function changed by this PR is currently preventing this from working correctly.

[yyx990803]

I'm not sure what you mean by

This turns silent errors into nice verbose errors

Can you give an example? I don't think it makes sense to change Vue internals to cater to a specific userland use case.

[neelance]

A simple example is a v-model="field.typoHere", with typoHere not existing. If the object is sealed, then writing to typoHere will trigger an error. If the object is not sealed, then this will silently succeed, but it will not have the intended behavior == bug. I prefer an error in this situation, because it can be tracked.

The current code has the assumption "a sealed object can never be reactive". Imho this is an assumption that does not hold universally. I don't see that as a specific use case. It is rather an unnecessary constraint that Vue currently has.

[yyx990803]

The extensible check was added to avoid unnecessary perf cost of traversing objects that are known to be frozen. Are you trying to create objects that are frozen only on itself, but allowing deep mutations inside? I feel that really complicates things.

FYI in 2.5+ v-model="foo.bar" creates bar as a reactive property if not already present so it no longer fails silently.

[neelance]

FYI in 2.5+ v-model="foo.bar" creates bar as a reactive property if not already present so it no longer fails silently.

It still fails silently if the property name is wrong. It sets the new property and makes it reactive, but this is not the property the developer intended to set.

Are you trying to create objects that are frozen only on itself, but allowing deep mutations inside?

No. I am not freezing the objects, I only seal them, so new properties can not be added but existing ones can be modified.

What about using Object.isFrozen instead of Object.isExtensible? I think isFrozen is what you want for performance reasons.

[neelance]

I have changed the PR accordingly. Please take another look. Is this better?

[yyx990803]

Object.isFrozen returns true on a sealed object as well, I don't think that would fix your use case.

[neelance]

You probably tested it on an empty object, which I also did initially. A sealed empty object is effectively frozen, since there is no property that could change. On non-empty objects it works as expected:

o1 = {};
Object.seal(o1);
console.log(Object.isSealed(o1), !Object.isExtensible(o1), Object.isFrozen(o1));
// => true true true

o2 = {answer: 42};
Object.seal(o2);
console.log(Object.isSealed(o2), !Object.isExtensible(o1), Object.isFrozen(o2));
// => true true false