Skip to content

Change additionalProperties default #612

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

Merged
merged 1 commit into from
May 25, 2021
Merged

Change additionalProperties default #612

merged 1 commit into from
May 25, 2021

Conversation

drwpow
Copy link
Contributor

@drwpow drwpow commented May 25, 2021

Resolves #607. A recent change (#585) allowed for the automatic addition of additional properties, so & { [key: string]: any } was added to all schema objects. The intent was to better reflect OpenAPI’s specification, and there is precedent for that. However, based on feedback, and previous expectations this library set, it was a little too disruptive.

And so, this PR reverts that behavior, turning this off by default, but adding a new --additional-properties` flag to enable this behavior (outlined in the README). This is a good compromise of allowing that behavior, but requiring it to be opt-in (which is also probably better to be stricter by default).

required: Object.keys(schema),
version,
})}\n}`;
output.definitions = transformSchemaObjMap(schema, { ...ctx, required });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing transformAll()’s default output to be an object rather than a string is required for the upcoming #602. But I wanted to get that change in before we release v4, so that v3 patches will be easier to work on (less codebase distance between v3 and v4)

@codecov
Copy link

codecov bot commented May 25, 2021

Codecov Report

Merging #612 (2d42dc8) into main (a36a728) will increase coverage by 1.61%.
The diff coverage is 88.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #612      +/-   ##
==========================================
+ Coverage   88.53%   90.15%   +1.61%     
==========================================
  Files           9       10       +1     
  Lines         349      386      +37     
  Branches      123      140      +17     
==========================================
+ Hits          309      348      +39     
- Misses         37       38       +1     
+ Partials        3        0       -3     
Impacted Files Coverage Δ
src/transform/headers.ts 23.07% <0.00%> (-4.20%) ⬇️
src/transform/index.ts 87.93% <78.78%> (+8.30%) ⬆️
src/transform/responses.ts 84.21% <81.81%> (-3.89%) ⬇️
src/transform/paths.ts 92.00% <87.50%> (ø)
src/transform/request.ts 95.00% <95.00%> (ø)
src/transform/parameters.ts 98.00% <96.42%> (+0.50%) ⬆️
src/index.ts 79.31% <100.00%> (+6.58%) ⬆️
src/transform/operation.ts 100.00% <100.00%> (+3.12%) ⬆️
src/transform/schema.ts 98.73% <100.00%> (+1.67%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31527ed...2d42dc8. Read the comment docs.

@drwpow drwpow merged commit 6969f9f into main May 25, 2021
@drwpow drwpow deleted the addl-props branch May 25, 2021 21:49
@drwpow drwpow mentioned this pull request May 27, 2021
gr2m added a commit to octokit/openapi-types.ts that referenced this pull request May 28, 2021
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 this pull request may close these issues.

New additionalProperties default breaks oneOf
1 participant