bcherny / json-schema-to-typescript

Compile JSON Schema to TypeScript type declarations
https://bcherny.github.io/json-schema-to-typescript-browser/
MIT License
2.92k stars 390 forks source link

[bug] $ref resolve uses wrong root dir #324

Open Hulkmaster opened 4 years ago

Hulkmaster commented 4 years ago

So we have project with structure like

project/apidocs/entitity project/apidocs/response project/ts-apidocs/entitity project/ts-apidocs/response

if we launch cli tool like json2ts -i ./apidocs/response/**/*.json -o ts-apidocs we have a problem ResolverError: Error opening file ".../project/entity/Something-2.json" (notice how apidocs folder is missing)

we have a file ./apidocs/response/Something.json which has reference "$ref": "../entity/Something-2.json"

During resolvement cli tool uses process.cwd for resolving https://github.com/bcherny/json-schema-to-typescript/blob/master/src/resolver.ts#L12

but according to docs mentioned at https://github.com/bcherny/json-schema-to-typescript/blob/master/src/index.ts#L24

https://tools.ietf.org/id/draft-pbryan-zyp-json-ref-03.html#rfc.section.4

If the URI contained in the JSON Reference value is a relative URI, then the base URI resolution MUST be calculated according to [RFC3986], section 5.2. Resolution is performed relative to the referring document.

so it should be relative to referring document (Something.json in my example) and not process.cwd

So our current workaround: we use docs dir as cwd, but it will break as soon as we'll have nested folder structure

danielblignaut commented 3 years ago

Hi,

I currently use my only build script as a work around, such as below:


import { compile, compileFromFile } from 'json-schema-to-typescript'
import fs from 'fs'
import path from 'path'

(async function() {
    const files = [
        'models/services/activity/activity/activity-application-model.v1.json',
        'models/services/activity/activity/activity-database-model.v1.json',
        'models/services/activity/activity/activity-create-request-model.v1.json',
    ]

    const ROOT_PATH = path.join(__dirname, '../../schema-registry')

    const promiseArr = files.map((file)=> {
        const sourceFile = path.join(ROOT_PATH, file)
        const sourceFileArr = sourceFile.split('.')
        const sourceFileDir = path.dirname(sourceFile)
        let destFile = ''

        for(let i=0; i<sourceFileArr.length -2; i++) {
            destFile += sourceFileArr[i] + '.'
        }

        destFile += 'd.ts'

        compileFromFile(sourceFile, {
            cwd: sourceFileDir
        })
            .then(ts => fs.writeFileSync(destFile, ts))
    })

    await Promise.all(promiseArr)

    console.log('DONE')
})()

I like to explicitly define the files I want to build in the array as I don't want every json file built into .d.ts files but you could easily us fs to scan the directory recursively and return all json files into the array. It also gives you a little more flex as to where you want the destination to be, etc.

relief-melone commented 2 years ago

Any progress on this?

relief-melone commented 2 years ago

I think i found the error to this in the resolver.js. cwd is used to dereference to schema. But it should be the path of the schema that is beeing dereferenced. I think i'll be able to stitch togeter a pull request. As I am not familiar with the rest of the code though this should be investigated properly ;)

relief-melone commented 2 years ago

I think the test is also wrong. If my file structure (like in the test) looks like this

root/
├─ moreTypes/
│  ├─ referencingType.json
├─ referencedType.json

shouldnt my referencingSchema reference like this

{ "$ref" : "../ReferencedType.json" }

instead of

{"$ref" : "ReferencedType.json" }
relief-melone commented 2 years ago

https://github.com/bcherny/json-schema-to-typescript/pull/421

OnkelTem commented 2 years ago

So basically there is no way currently to reference schemas?

relief-melone commented 2 years ago

Sadly it looks like it. As no one seems to take a look at the pr and we were in need of the change I made a fork (which I still hope I can deprecate at some point as I really don't like this approach)

https://www.npmjs.com/package/rm-json-schema-to-typescript

The fork should do the trick though

bcherny commented 2 years ago

Sorry for the delayed response! Feel free to continue work on the PR to resolve requested changes.

lorefnon commented 2 years ago

The package rm-json-schema-to-typescript does not seem to resolve this issue esp. when array item contains a ref.

{
    properties: {
        events: {
            type: 'array',
            items: { $ref: 'models/Event.json', $id: '...' }
        }
    }
}

So basically there is no way currently to reference schemas?

A simple workaround is to pre-process the json schema and resolve all $ref paths to absolute before invoking this library.

rvanlaak commented 2 years ago

Changing all references to an absolute path makes the script work.

The next problem is that when multiple schemas have a ref to the same file, all output files have all referenced schemas duplicated as well. Can the output land in a single file?

relief-melone commented 2 years ago

The package rm-json-schema-to-typescript does not seem to resolve this issue esp. when array item contains a ref.

{
    properties: {
        events: {
            type: 'array',
            items: { $ref: 'models/Event.json', $id: '...' }
        }
    }
}

So basically there is no way currently to reference schemas?

A simple workaround is to pre-process the json schema and resolve all $ref paths to absolute before invoking this library.

try using

//....
items: { "$ref": "./models/Event.json" },
//...

That should work. I use that type of referencing in the fork without any problems for quite some time now

dhvector commented 2 years ago

The next problem is that when multiple schemas have a ref to the same file, all output files have all referenced schemas duplicated as well. Can the output land in a single file?

Keep an eye on #258 for that btw.

RE: the absolute paths, that's exactly what I had to do as well - in my case I have a node script that scans for $ref and swaps relative file paths with path.resolve(path.dirname(file), match). The jsonschema2pojo project handles relative paths nicely, in case that's useful as a data point.

gtrabanco commented 2 years ago

So basically there is no way currently to reference schemas?

  "scripts": {
    "build:schemas": "rm -rf ./types/schemas/*.d.ts && cd schemas && json2ts -i '*.json' -o ../types/schemas/"
  },

Due it is using cwd as work directory to build schemas the only thing you have to do until is fixed is build your schemas from the same directory so you can perform a cd command before build.