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

feat: add object and propName to onDereference callback #324

Merged

Conversation

toomuchdesign
Copy link
Contributor

@toomuchdesign toomuchdesign commented Sep 25, 2023

This is an implementation proposal to add 2 new arguments to dereference.onDereference options.

The 2 new arguments are:

  • parent: The parent of the dereferenced object
  • parentPropName: The prop name of the parent object whose value was dereferenced

In other words: the current 2nd onDereference argument (the inlined schema) would be the result of object[propName].

Extra info

Exposing a callback with 4 arguments is probably sub optimal in terms of API. In a next major release the callback could be refactored to accept an object to provide named arguments.

The idea behind this PR was born while developing openapi-ts-json-schema, where json-schema-ref-parser's onDereference callback is used to append meta data to the dereferenced objects. This would sensibly provide more flexibility and power to the callback.

I'm aware it could be quite niche use case and I'll be fine whatever decision you are going to take about this PR.
Happy to update this PR based on you feedback, if the case.

Thank you so much for the priceless work you are doing here! 🙌
Cheers!

@toomuchdesign toomuchdesign force-pushed the extend-ondereference-argument-list branch from d82a896 to 9f477b9 Compare September 25, 2023 21:06
@coveralls
Copy link

coveralls commented Mar 4, 2024

Pull Request Test Coverage Report for Build 8160456238

Details

  • 6 of 6 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.05%) to 95.851%

Totals Coverage Status
Change from base Build 8160379361: 0.05%
Covered Lines: 3219
Relevant Lines: 3318

💛 - Coveralls

@jonluca jonluca merged commit 4db8afe into APIDevTools:main Mar 5, 2024
14 checks passed
Copy link

github-actions bot commented Mar 5, 2024

🎉 This PR is included in version 11.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants