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

Wrong path on Windows #2052

Closed
mmospanenko opened this issue Mar 1, 2024 · 16 comments · Fixed by #2053 or hey-api/openapi-ts#30
Closed

Wrong path on Windows #2052

mmospanenko opened this issue Mar 1, 2024 · 16 comments · Fixed by #2053 or hey-api/openapi-ts#30
Assignees

Comments

@mmospanenko
Copy link

On Windows, when use app under another process, RefParser gets wrong path. And as result - it joins abs and rel paths from package and argument. So I had to fix it like this:
image

STR:

  1. create file with exec (cli or import - the same)
  2. run as node myfile.js on windows
image
@mrlubos
Copy link
Collaborator

mrlubos commented Mar 1, 2024

Thanks. Seems quite a few people run into this issue

@tajnymag
Copy link
Contributor

tajnymag commented Mar 2, 2024

All I had to do to fix it was to use the newest json-schema-ref-parser by overriding its depedency in package.json

"pnpm": {
    "overrides": {
      "@apidevtools/json-schema-ref-parser": "11.1.0"
    }
  }

@mrlubos
Copy link
Collaborator

mrlubos commented Mar 2, 2024

@tajnymag hm, this package used that version before and people still complained so it got reverted

@tajnymag
Copy link
Contributor

tajnymag commented Mar 2, 2024

@tajnymag hm, this package used that version before and people still complained so it got reverted

Interesting. I'll try to find out why it worked and why it would't. I tried it with openapi-codegen (your version) installed as a devDependency

@mrlubos
Copy link
Collaborator

mrlubos commented Mar 2, 2024

@tajnymag thanks, let me know if you find out please. I didn't get around to trying this one out yet

@tajnymag
Copy link
Contributor

tajnymag commented Mar 2, 2024

@tajnymag thanks, let me know if you find out please. I didn't get around to trying this one out yet

So far, I can only say original and fork behave the same in regard to this issue and workaround.

@mrlubos
Copy link
Collaborator

mrlubos commented Mar 2, 2024

@tajnymag i suspect we'll need to fork the other package too APIDevTools/json-schema-ref-parser#303

@tajnymag
Copy link
Contributor

tajnymag commented Mar 2, 2024

@mrlubos I think I fixed the issue. Basically followed @mmospanenko's suggestion and made all passed paths to json-schema-ref-parser absolute.

Both of these had to be made together to pass our tests

  1. always pass absolute url to json-schema-ref-parser
  2. update json-schema-ref-parser to v11.1.0

@mrlubos
Copy link
Collaborator

mrlubos commented Mar 2, 2024

@tajnymag is the absolute path conversion happening inside this package or is that something you do before passing the value to --input?

edit: I see! Do you want to open a pull request against our fork?

@mrlubos
Copy link
Collaborator

mrlubos commented Mar 2, 2024

@mmospanenko we've published a fix in our fork, mind giving it a try and see if it works without the need for workarounds on your end?

@mmospanenko
Copy link
Author

I can't test on Windows, it was funny testing game while we were trying to solve the issue)) but yes, will be great to remove workaround, thank you, @mrlubos!
I think thread could be closed if you've fixed this

@mrlubos
Copy link
Collaborator

mrlubos commented Mar 2, 2024

@mmospanenko i don't have the permissions to close issues in this repository, but as long as our fix works for you, I will continue referring people to our fork if they experience the same issue

@o0Maikeru0o
Copy link

@mrlubos Your fork hasn't fixed this issue for me on windows. Any version after json-schema-ref-parser@9.0.9 is causing this problem

@tajnymag
Copy link
Contributor

tajnymag commented Mar 4, 2024

@mrlubos Your fork hasn't fixed this issue for me on windows. Any version after json-schema-ref-parser@9.0.9 is causing this problem

Can you share with us a small repo reproducing the problem? The commited fix did fix the tests on my machine. Perhaps there's a case I overlooked and did not test.

@o0Maikeru0o
Copy link

@tajnymag That was my mistake. I hadn't updated the package to the latest version with your changes. Seems to working now. Thanks for contributing

@mrlubos
Copy link
Collaborator

mrlubos commented Mar 4, 2024

Cool stuff @tajnymag, thanks for confirming @o0Maikeru0o

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