Feature: Append file extension based on runtime #936
Conversation
|
|
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
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 |
|
Hey @mrlubos , whichever the extension of the file is. Im not sure how the runtime is detected, I can check on the Deno docs. |
|
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. |
|
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 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 |
|
Deno has come a long way to resolve imports from 
And it just works, I can generate code from deno no issues. |
|
@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? |
|
@mrlubos The only reason I generated from node is because I use Generation currently works for me in both Deno and Node and produce the same outputs |
bombillazo
changed the title
|
@mrlubos I updated the PR to reflect your feedback of making it dynamic |
| "global.d.ts", | ||
| "./src/**/*.ts", | ||
| "rollup.config.ts", | ||
| "rollup.dts.config.ts" |
There was a problem hiding this comment.
Why do you need to add this file?
| 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); |
There was a problem hiding this comment.
Do we need to add extension to node_modules imports too? This may generate code like '@hey-api/client-fetch.ts'
There was a problem hiding this comment.
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...
| @@ -0,0 +1,15 @@ | |||
| // global.d.ts | |||
There was a problem hiding this comment.
Is this file for the utils/runtime module?
There was a problem hiding this comment.
Yes, if not TypeScript (node) thinks it is an undeclared type since it is not aware of Deno and Bun (rightfully so)
|
Hi guys, thanks for the effort! Just chiming in that the you also need file extensions when compiling a project with But instead of |
|
It would be better to have it configurable without runtime detection. In my case I'm running my project with |
Yup, I agree, in our case it was due to building it for Deno, which requires extensions, but we're running it from node. |
|
@bombillazo Would you be able to make the tests pass? |
Fixes #935
Adds a new options to
outputto specify file extensions to generate