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

Permit null as an enum value in referenced schemas - #59

Merged
kylef merged 2 commits into
masterfrom
kylef/ref-nullable-enum
Jan 10, 2019
Merged

kylef merged 2 commits into
masterfrom
kylef/ref-nullable-enum

Conversation

@kylef

@kylef kylef commented Jan 10, 2019 •

Copy link
Copy Markdown
Member

The logic to check if we need to dereference a schema was failing in a case where we would find null as a value on an enum. As we're checking for the presence of null.$ref which causes an error to be thrown:

Cannot read property '$ref' of null

This is a little tricky to test as it is a combination of things happen and thus I have not created a unit regression test but only a regression integration fixture. This problem can only happen when you reference another schema, for example the following tests pass without these fixes:

it('translates x-nullable to enum value', () => {
  const schema = convertSchema({ enum: ['north', 'east', 'south', 'west'], 'x-nullable': true });
  expect(schema).to.deep.equal({ enum: ['north', 'east', 'south', 'west', null] });
});

it('translates x-nullable to enum value with existing null in enum', () => {
  const schema = convertSchema({ enum: ['north', 'east', 'south', 'west', null], 'x-nullable': true });
  expect(schema).to.deep.equal({ enum: ['north', 'east', 'south', 'west', null] });
});

It's only when convertSchema is used when the schema references another schema where you'd be able to reproduce.

kylef added 2 commits January 10, 2019 14:39
The logic to check if we need to dereference a schema was failing in a
case where we would find `null` as a value on an enum. As we're checking
for the presence of `null.$ref` which causes an error to be thrown:

> Cannot read property '$ref' of null
@kylef
kylef merged commit 63e4e95 into master Jan 10, 2019
@kylef
kylef deleted the kylef/ref-nullable-enum branch January 10, 2019 15:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants