Feature: Append file extension based on runtime

[bombillazo]

Fixes #935

Adds a new options to output to specify file extensions to generate

[bolt-new-by-stackblitz[bot]]

Run & review this pull request in StackBlitz Codeflow.

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: 8512ed3a506a3664e4f39c0adda7f2dc9272253e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
hey-api-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 15, 2024 3:20am

[mrlubos]

Hey @bombillazo, which extension do you need to run this with your project? Can you not detect Deno runtime from within code? Not a fan of configuration, this should be automatically detected imo. We'll also need to add tests for Deno runtime as everything runs inside Node.js right now

[bombillazo]

Hey @mrlubos , whichever the extension of the file is. Im not sure how the runtime is detected, I can check on the Deno docs.

[bombillazo]

I agree autodetected would be ideal, although it will prevent the capacity to generate Deno-compatible code from node or make node compatible code from Deno.

If it's too much overhead for Deno, we can keep discussing and I'll just modify the code generated post-gen on our project for now.

[mrlubos]

Yeah that would be the first thing to resolve. We want to support Bun too (I am not sure if anything is missing right now, haven't looked into it yet). I am surprised you don't need to abstract platform-specific modules such as node:fs, how come?

Also, I am 100% sure there are missed imports, for example services import from types (EDIT: okay not 100% sure, maybe you got them all haha). In my opinion this should be abstracted inside the TypeScriptFile class and let it handle extensions. I don't worry too much about supporting the old code, i.e. Handlebars templates. But it needs to work seamlessly with the new clients without extra configuration, I think that's all achievable

[bombillazo]

Deno has come a long way to resolve imports from npm:, node: etc out of the box, I run the createClient function from a ts file with deno run and import like this:

And it just works, I can generate code from deno no issues.

[mrlubos]

@bombillazo yeah there's a reason I didn't tackle it yet. I don't want to hastily add features because people will start using them and then modifying any mistakes becomes painful. Are you generating Deno-compatible code from Node.js? If so, why?

[bombillazo]

@mrlubos The only reason I generated from node is because I use patch-package which only works for node, I forked this repo, build the new version and copied the dist to my other project to test the code generation. If it works I create a patch so anyone in our repo can use it. I have not found a way to do that in Deno.

Generation currently works for me in both Deno and Node and produce the same outputs

[bombillazo]

@mrlubos I updated the PR to reflect your feedback of making it dynamic

[simonnepomuk]

Hi guys,

thanks for the effort! Just chiming in that the you also need file extensions when compiling a project with tsc and the following tsconfig.json:

    "module": "NodeNext",
    "strict": true,
    "target": "ES2023",
    "moduleResolution": "NodeNext"

But instead of .ts it needs to be .js. I would imagine that this is not an uncommon use case. Would be happy to assist :)

[jgoux]

It would be better to have it configurable without runtime detection. In my case I'm running my project with node --experimental-strip-types and the TypeScript option "allowImportingTsExtensions": true, so I would need to manually indicate that I want .ts extensions in my imports and exports.

[bombillazo]

It would be better to have it configurable without runtime detection. In my case I'm running my project with node --experimental-strip-types and the TypeScript option "allowImportingTsExtensions": true, so I would need to manually indicate that I want .ts extensions in my imports and exports.

Yup, I agree, in our case it was due to building it for Deno, which requires extensions, but we're running it from node.

[mrlubos]

@bombillazo Would you be able to make the tests pass?

[bombillazo]

I'll have to revisit my code; I haven't really updated my fork since I'm very busy, and it is working for our needs, but it seems like it's diverged/ahead by a lot.

If I did come back to this its hopefully to resolve our use case with eh native library and stop using our fork.

[mrlubos]

No worries @bombillazo, this will get done one way or another. If you'd be able to update the pull request, great, if not that's okay too. Glad it's working for you so far!

[bombillazo]

Ill try to revisit this with the latest changes

[mrlubos]

@bombillazo sounds good! Did you see there's now a built-in capability to append .js depending on tsconfig setup? I wonder if you could extend or repurpose that implementation to achieve this

https://heyapi.dev/openapi-ts/migrating#v0-67-0

[mrlubos]

I'm going to close this pull request. I think we could create an issue to add .ts extensions for Deno environments, it wouldn't even have to be an explicit configuration option. I imagine we'd have an option like environment and when set to 'deno', we'd automatically append .ts

[bombillazo]

Sounds good, yeah sorry I kinda dropped the ball with this, too busy with lots of stuff, but we could keep #935 to track this. Im not sure if other runtimes beside Deno require file extensions, so maybe having it a s a generic option works.

One thing to consider is that the JS runtime one uses to build the API SDK is not necessarily the one used to consume/use the SDK, for example, we had cases where we were using Node to run openapi-ts to build an SDK for Deno.