APIDevTools / json-schema-ref-parser

Parse, Resolve, and Dereference JSON Schema $ref pointers in Node and browsers
https://apitools.dev/json-schema-ref-parser
MIT License
952 stars 227 forks source link

Calling dereference mutates input schema, while example does not indicate this #258

Closed Swiftwork closed 6 months ago

Swiftwork commented 2 years ago

The example section seems to indicate that a new schema object with be created with the dereferenced values, but in reality, it mutates mySchema as well. Either the dereference command shouldn't mutate the original object or the example should be updated to reflect reality.

https://github.com/APIDevTools/json-schema-ref-parser#example

try {
  let schema = await $RefParser.dereference(mySchema);
  console.log(schema.definitions.person.properties.firstName);
}
catch(err) {
  console.error(err);
}
philsturgeon commented 2 years ago

Could you send a PR with a failing test showing the mutation happening? Then it will be much easier for somebody to take a swing at fixing it.

Swiftwork commented 2 years ago

Hi @philsturgeon, thanks for the suggestion. I put together a codepen example https://codesandbox.io/s/node-playground-forked-k4vvz9?file=/src/index.js to demonstrate what is happening as this was easier. What this issue is really referring to is that:

let schema = await $RefParser.dereference(mySchema);
// and
await $RefParser.dereference(mySchema);

Both are performing the same operation, mutating the original object mySchema, the only difference in the first example being that mySchema is also aliased to schema. In my opinion, mutation is harder to understand from a coding perspective and can cause some side effects, so it is better to avoid it. But if that's not possible or easily achieved, then the documentation should be clear :). If no code changes are made then the example should probably be updated to:

try {
  await $RefParser.dereference(schema);
  console.log(schema.definitions.person.properties.firstName);
}
catch(err) {
  console.error(err);
}
giorgosera commented 1 year ago

I stumbled upon the same thing and indeed it'd be useful if the documentation indicated that the input schema is mutated or if the mutation didn't happen in the first place.

jonluca commented 6 months ago

Great idea, included this as a flag in the latest version to preserve the historical behavior. I also updated the readme to better reflect the mutation in place