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

[REQ] New typescript-generator option to set file extension for ECMAScript module resolution #14333

Closed
danmichaelo opened this issue Dec 27, 2022 · 3 comments · Fixed by #14371

Comments

@danmichaelo
Copy link
Contributor

danmichaelo commented Dec 27, 2022

Is your feature request related to a problem? Please describe.

I'm trying to generate a pure ESM package (type: "module") using ESM in Node.js, but the Typescript generators generate relative imports without file extensions (e.g. import * as runtime from "../runtime") while Node.js requires file extensions for ESM imports (import * as runtime from "../runtime.js")

Describe the solution you'd like

  • The typescript generator already has an internal option extensionForDeno, which is set to .ts when platform is deno. I suggest renaming extensionForDeno to importFileExtension and making it a public option.
  • The other typescript generators does not have such an option, so it may have to be added to those, if needed

I can try putting together a PR for this, but would like to gather some feedback about whether it's a good idea first.

Describe alternatives you've considered

  • It's possible to generate a valid ESM package by setting moduleResolution: "node" (CommonJS for internal module resolution) and module: "ESNext", but this will fail if we add additional dependencies to the generated package that aren't CommonJS compatible.

  • Instead of adding a new option importFileExtension, derive the file extension from some other option:

    • From platform? If we add a new value like node-esm. The problem is that there are several different extensions that can potentially be used (.js, .mjs, .tjs etc.) and users may have different preferences.
    • Perhaps a new option moduleResolution with values node and node16 inspired by TypeScript?
@stefanpl
Copy link

stefanpl commented Jan 1, 2023

Hi @danmichaelo (and a Happy New Year 🎉),

I'm currently facing the same issue. importFileExtension as a public option sounds straightforward to me from a DX perspective 👍

My understanding was that the typescript generator is meant to focus all TS-related efforts, so adding it there should probably be the main goal. Until this lands, I'll post our current workaround for typescript-fetch below.

@stefanpl
Copy link

stefanpl commented Jan 1, 2023

Our team's current workaround for the typescript-fetch generator (which we're using since typescript is still flagged "experimental"):

  • we're using npx fix-esm-import-path generated/dir to add the extension to the generated files.
  • the only thing not catched by this is the import type ... from '../models', so we slightly changed that with a modified template (touching only apis.mustache, and sagas.mustache if need be):
-} from '../models';
+} from '../models/index.js';

@danmichaelo
Copy link
Contributor Author

Thanks @stefanpl – and happy new year ✨

npx fix-esm-import-path was neat – I wasn't aware of that one!

I put together a PR for typescript to start with, but we're also using typescript-fetch at the moment, so perhaps I'll try to put together a second PR for that if the first is accepted.

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