Skip to content
This repository was archived by the owner on Nov 8, 2024. It is now read-only.

Prevent throwing during freeze for required annotations #502

Merged
merged 1 commit into from
Jun 24, 2020

Conversation

kylef
Copy link
Member

@kylef kylef commented Jun 23, 2020

The parser can create multiple errors from an object which contain the exact same instance of a source map element. Thus creating integration problems using the parse result as the same element instance is re-used and thus a modification to one causes modification to others. This can be demonstrated by using .freeze() method as that requires each element to only be used a single time within the tree.

Prevents the error from being throw to users as per apiaryio/dredd#1762

@@ -141,6 +144,13 @@ describe('#parseObject', () => {
"'Example Object' is missing required property 'required1'",
"'Example Object' is missing required property 'required2'",
]);

// assert errors can be frozen (we're testing that the source map
// for object was cloned instead of referenced)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity how would it look like without the clone? It is not completely obvious for me how this is testing the not-a-reference.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

freeze() would throw the following error:

Cannot assign to read only property 'parent' of object '#'

Copy link
Member Author

@kylef kylef Jun 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s because effectively it’s doing this:

freeze() {
  freeze “this”
  
  for element in (all meta, attributes, content):
    element.parent = this
    element.freeze()
}

Where a re-used element would fail as we can’t set .parent on the second time around

Copy link
Contributor

@opichals opichals left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@kylef kylef merged commit 2d5cdfd into master Jun 24, 2020
@kylef kylef deleted the kylef/freeze-object branch June 24, 2020 08:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working openapi3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants