-
-
Notifications
You must be signed in to change notification settings - Fork 506
Support ES Module import in config file #2510
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
Conversation
|
@melloware fixed the |
5d0ac49 to
338c134
Compare
|
@melloware fixed the issues with |
|
Running now. |
|
@melloware the failing tests looks the same as the ones for another PR that was merged today https://github.com/orval-labs/orval/actions/runs/18918526841/job/54009465663 |
|
@Noyabronok yep i am investigating today |
|
@melloware since those tests were already broken, does this PR need to wait for them to be fixed? The config file processing changes in this PR have passed their checks. Can we merge? |
|
Nope nothing i need you to do yet. |
|
ok i fixed |
338c134 to
e997f71
Compare
|
@melloware done |
|
@melloware Thanks! I tested this in v8rc.1 and it works well. However, there are several other breaking changes in version 8. Is there a process to apply changes in this PR to 7.15 to ease upgrade pains, while version 8 is baking? |
|
@Noyabronok i just created a v7 branch if you want to submit a PR against that I can try and release 7.16.0 https://github.com/orval-labs/orval/tree/v7 |
awesome! I did figure out how to get the old behavior back in v8... |
|
Yep we have a migration guide: https://orval.dev/guides/migration-v8 let us know if we missed anything and we can add it. |
hmm..I wish I saw this migration guide before. I think with the jiti changes in this PR the .mjs or type: modules requirements are not necessary. I still have my config file as orval.config.ts and package.json still using commonjs type without issues. |
The fix
_fakerinstead offakerNote: this fix is needed for orval versions 8 and 7.14-7.15
Background:
Version 7.14 was a breaking change requiring Node 22. It broke ESM module loading. If you run the orval tests in this PR without jiti you'd get errors like
Alternative solutions
Without this fix, the user can specify NODE_OPTIONS to process the imports. For example, this is how you'd process with tsx:
yarn generate:multi-file// erroryarn generate:multi-file:workaround// pass