Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error thrown on Proxys with undescribed own properties #71

Closed
ninevra opened this issue Mar 22, 2021 · 3 comments · Fixed by #72
Closed

Error thrown on Proxys with undescribed own properties #71

ninevra opened this issue Mar 22, 2021 · 3 comments · Fixed by #72

Comments

@ninevra
Copy link
Contributor

ninevra commented Mar 22, 2021

compare(), format(), serialize(), and diff() throw an error when called on objects which have own properties with no property descriptors. (As far as I know, the only such objects are Proxys.)

concordance assumes that, if Object.getOwnPropertyNames(input) returns e.g. ['name'], then Object.getOwnPropertyDescriptor(input, 'name') will return a property descriptor. This is true for non-Proxy inputs, but can be false for Proxys, which in most cases don't have to respect any relation between the ownKeys and ownPropertyDescriptor operations.

This can lead to throwing

TypeError {
  message: 'Cannot read property \'enumerable\' of undefined',
}

at

if (accept && Object.getOwnPropertyDescriptor(obj, name).enumerable) {

I encountered this when I accidentally tried to snapshot a jsdom Window object, which apparently has some ownKeys that lack descriptors.

I'm not sure whether there's a better way to handle this. The "value" of the property can still be retrieved with e.g. Reflect.get(), so that could be used for comparison; but enumerability and other property attributes are strange.

As far as I can tell, undescribed properties do not behave identically to any valid property descriptor, nor to the absence of a property. They are not themselves listed in enumerations, but neither do they mask the presence of enumerable prototype properties, as truly unenumerable properties do. They may or may not satisfy Reflect.has(object, property).

@novemberborn
Copy link
Member

Should we ignore these? We already ignore non-enumerable properties.

@ninevra
Copy link
Contributor Author

ninevra commented Mar 31, 2021

That makes sense to me. These properties certainly aren't enumerable, which seems close enough to non-enumerable to similarly ignore.

@rrthomas
Copy link

Please can we have a release that includes this fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants