Use $ref instead of allOf for Extending Closed Schemas #65

[29deepanshutyagi]

This pull request updates the instructions.mdx file to use $ref instead of allOf for extending closed schemas. The changes include:

Explanation of the limitations of additionalProperties.
Demonstration of how to use $ref to extend the address schema by adding a type property.
Use of unevaluatedProperties to control additional properties.
Changes
Updated instructions.mdx to reflect the correct usage of $ref and unevaluatedProperties.
Related Issue
Fixes #65: Use $ref instead of allOf for Extending Closed Schemas

[29deepanshutyagi]

kindly review it @JeelRajodiya

[Chromosore]

I come from the tour and the original page was a bit confusing so I think this PR makes it better and clearer, thanks. However I feel like the original title is not correct anymore. Is the address schema closed ? Or is the outer schema closed ? (and can a schema closed with additionalProperties: false even be extended ? should there be a comment or another page about this ?)

Also, not related, i feel like the address schema should be referenced with the actual path #/definitions/address and not a made up path like example.org because I find it more grounded and easy to understand for a beginner like me

[jdesrosiers]

This needs a lot of work. I left comments inline. I didn't comment on the second schema in the lesson. It has the same issues as the first schema, so please make the fixes in both places.

I don't think it makes sense that the solution to this lesson is in the lesson itself. The user just needs to copy paste the schema into the editor. Either the lesson should use a different example or the task should be different.

My inline comments make this work, but it's not the solution the issue describes. It should be providing an external address schema that the user then needs to extend. Defining the address schema as a definition is better than it is now, but that wasn't the task.

[jdesrosiers]

Suggested change

	So, `additionalProperties` can restrict you from "extending" a schema using combining [keywords](https://json-schema.org/learn/glossary#subschema) such as `$ref`. In the following example, we can see how the `additionalProperties` can cause attempts to extend the address schema example to fail.
	So, `additionalProperties` can restrict you from "extending" a schema using combining [keywords](https://json-schema.org/learn/glossary#subschema) such as `$ref`. In the following example, we can see how `additionalProperties` can cause attempts to extend the address schema example to fail.

[jdesrosiers]

This sentence doesn't make sense. It's certainly not a description of unevaluatedProperties.

Suggested change

	The challenge we saw with `additionalProperties` can be solved using the `unevaluatedProperties` keyword. This keyword allows you to define properties that are not evaluated by the current schema.
	The problem we saw with `additionalProperties` can be solved using the `unevaluatedProperties` keyword. This keyword allows you to forbid any properties that are not evaluated by the current schema _or_ any schemas applied to the same location using the combining keywords.

This version might be too technical to make sense to someone trying to learn JSON Schema, but at least its correct. If someone can make it more accessible, please help.

[jdesrosiers]

You didn't update the line highlight when you changed the schema.

Suggested change

	```json highlightLineStart={11}
	```json highlightLineStart={7}

[jdesrosiers]

This doesn't make sense. What are you expecting this to reference? This can be made to make sense if you reference the address definition.

Suggested change

	"$ref": "https://example.com/address",
	"$ref": "#/$defs/address",

[jdesrosiers]

This tour uses 2020-12.

Suggested change

	"definitions": {
	"$defs": {

[jdesrosiers]

Remove this whitespace.

[jdesrosiers]

I don't know what you were trying to do with this, but it doesn't make sense. It should be something like the following.

const code: any = {
  $ref: "#/$defs/address",
  properties: {
    type: { enum: ["residential", "business"] },
  },
  required: ["type"],
  $defs: {
    "address": {
      type: "object",
      properties: {
        street_address: { type: "string" },
        city: { type: "string" },
        state: { type: "string" },
      },
      required: ["street_address", "city", "state"]
    }
  }
};

[jdesrosiers]

This is unnecessary and too much whitespace.

[jdesrosiers]

What is this for? Seems unnecessary.

[jdesrosiers]

Add unevaluatedProperties to the schema to allow only number as an additional property.

That's not actually the task that's expected.