fix: ObjectSchema.extend returns a schema instance

fastify / fluent-json-schema

A fluent API to generate JSON schemas

MIT License

495 stars 58 forks source link

fix: ObjectSchema.extend returns a schema instance #253

Open deepaknf opened 1 month ago

deepaknf commented 1 month ago

Summary

This PR addresses the issue https://github.com/fastify/fluent-json-schema/issues/249 where ObjectSchema.extend Does not return a schema instance. The fix makes it return the schema.

Checklist

[x] run npm run test
[x] tests and/or benchmarks are included
[ ] documentation is changed or added
[x] commit message and code follows the Developer's Certification of Origin and the Code of conduct

bencoder commented 1 month ago

It seems it was intentionally not returning a Schema instance before this. Do we know why it was like that in the first place?

Looking at where that was originally changed (to not return Schema) I see this PR & issue with some explanation but I don't understand the context well enough to say if it's still valid:

https://github.com/fastify/fluent-json-schema/pull/69 https://github.com/fastify/fluent-json-schema/issues/65

kieranswhite commented 1 month ago

@aboutlo I don't suppose you can help us understand this? It looks like you were heavily involved in the previous issues that @bencoder listed. Thanks :)

simoneb commented 1 month ago

I don't think we can safely merge this change, precisely because, as observed, this behavior is intentional. The change is straightforward, but before we even change anything, we need to understand why the current behavior was intentionally implemented in this way, and the explanation lies in the PRs linked by @bencoder above. So, please have a look at the whole history before deciding what to do with this issue. A viable option is to close it as it works as designed, but I would consider giving another look at the original motivations behind this behavior, to see if there's a solution that wasn't considered back then.