Feature: Append file extension based on runtime #936

bombillazo · 2024-08-14T19:53:52Z

Fixes #935

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

stackblitz · 2024-08-14T19:53:55Z

Run & review this pull request in StackBlitz Codeflow.

changeset-bot · 2024-08-14T19:53:55Z

⚠️ No Changeset found

Latest commit: 8512ed3

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 · 2024-08-14T19:53:56Z

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 · 2024-08-14T20:02:52Z

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 · 2024-08-14T20:07:27Z

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 · 2024-08-14T20:15:04Z

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 · 2024-08-14T20:15:20Z

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 · 2024-08-14T20:19:38Z

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 · 2024-08-14T20:20:03Z

@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 · 2024-08-14T20:21:21Z

@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

…e templates

bombillazo · 2024-08-15T03:17:33Z

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

mrlubos · 2024-08-15T06:06:37Z

packages/openapi-ts/tsconfig.json

+    "global.d.ts",
+    "./src/**/*.ts",
+    "rollup.config.ts",
+    "rollup.dts.config.ts"


Why do you need to add this file?

mrlubos · 2024-08-15T06:07:33Z

packages/openapi-ts/src/generate/client.ts

 import { ensureDirSync } from './utils';

 const require = createRequire(import.meta.url);

 export const clientModulePath = () => {
  const config = getConfig();
-  return config.client.bundle ? './client' : config.client.name;
+
+  return appendExt(config.client.bundle ? './client' : config.client.name);


Do we need to add extension to node_modules imports too? This may generate code like '@hey-api/client-fetch.ts'

No, only for relative/absolute paths or importing specific files.

I see, Id have to check then how to differentiate if its coming from core or an external package...

mrlubos · 2024-08-15T06:08:34Z

packages/openapi-ts/global.d.ts

@@ -0,0 +1,15 @@
+// global.d.ts


Is this file for the utils/runtime module?

Yes, if not TypeScript (node) thinks it is an undeclared type since it is not aware of Deno and Bun (rightfully so)

simonnepomuk · 2024-09-16T21:14:25Z

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 · 2024-10-17T07:45:39Z

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 · 2024-10-17T12:18:17Z

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 · 2024-10-17T12:24:06Z

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

bombillazo · 2024-12-27T13:58:10Z

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 · 2024-12-27T15:23:44Z

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!

add extension option to client

a193017

vercel bot deployed to Preview August 14, 2024 19:54 View deployment

bombillazo mentioned this pull request Aug 14, 2024

Support Deno #554

Open

bombillazo added 5 commits August 14, 2024 23:02

chore: add utils to check which runtime is being used

a7cc9f4

chore: add util to append extension based on runtime

9e3408f

chore: update usage of appendExt function

06a4b95

chore: remove extension config option

3092cf0

chore: add getExt function and use it to fill template context, updat…

1f04b9b

…e templates

bombillazo changed the title ~~Feature: Add extension option to client~~ Feature: Append file extension based on runtime Aug 15, 2024

vercel bot deployed to Preview August 15, 2024 03:17 View deployment

chore: remove unused extension property in config init

8512ed3

vercel bot deployed to Preview August 15, 2024 03:20 View deployment

mrlubos reviewed Aug 15, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Append file extension based on runtime #936

Feature: Append file extension based on runtime #936

bombillazo commented Aug 14, 2024

stackblitz bot commented Aug 14, 2024

changeset-bot bot commented Aug 14, 2024 •

edited

Loading

vercel bot commented Aug 14, 2024 •

edited

Loading

mrlubos commented Aug 14, 2024

bombillazo commented Aug 14, 2024

bombillazo commented Aug 14, 2024

mrlubos commented Aug 14, 2024 •

edited

Loading

bombillazo commented Aug 14, 2024

mrlubos commented Aug 14, 2024

bombillazo commented Aug 14, 2024 •

edited

Loading

bombillazo commented Aug 15, 2024

mrlubos Aug 15, 2024

mrlubos Aug 15, 2024

bombillazo Aug 15, 2024

mrlubos Aug 15, 2024

bombillazo Aug 15, 2024

simonnepomuk commented Sep 16, 2024

jgoux commented Oct 17, 2024

bombillazo commented Oct 17, 2024

mrlubos commented Oct 17, 2024

bombillazo commented Dec 27, 2024

mrlubos commented Dec 27, 2024

Feature: Append file extension based on runtime #936

Are you sure you want to change the base?

Feature: Append file extension based on runtime #936

Conversation

bombillazo commented Aug 14, 2024

stackblitz bot commented Aug 14, 2024

changeset-bot bot commented Aug 14, 2024 • edited Loading

⚠️ No Changeset found

vercel bot commented Aug 14, 2024 • edited Loading

mrlubos commented Aug 14, 2024

bombillazo commented Aug 14, 2024

bombillazo commented Aug 14, 2024

mrlubos commented Aug 14, 2024 • edited Loading

bombillazo commented Aug 14, 2024

mrlubos commented Aug 14, 2024

bombillazo commented Aug 14, 2024 • edited Loading

bombillazo commented Aug 15, 2024

mrlubos Aug 15, 2024

Choose a reason for hiding this comment

mrlubos Aug 15, 2024

Choose a reason for hiding this comment

bombillazo Aug 15, 2024

Choose a reason for hiding this comment

mrlubos Aug 15, 2024

Choose a reason for hiding this comment

bombillazo Aug 15, 2024

Choose a reason for hiding this comment

simonnepomuk commented Sep 16, 2024

jgoux commented Oct 17, 2024

bombillazo commented Oct 17, 2024

mrlubos commented Oct 17, 2024

bombillazo commented Dec 27, 2024

mrlubos commented Dec 27, 2024

changeset-bot bot commented Aug 14, 2024 •

edited

Loading

vercel bot commented Aug 14, 2024 •

edited

Loading

mrlubos commented Aug 14, 2024 •

edited

Loading

bombillazo commented Aug 14, 2024 •

edited

Loading