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

[Request] Add .js file extensions to generated imports #5524

Closed
3 of 10 tasks
timbrinded opened this issue Mar 7, 2023 · 6 comments
Closed
3 of 10 tasks

[Request] Add .js file extensions to generated imports #5524

timbrinded opened this issue Mar 7, 2023 · 6 comments

Comments

@timbrinded
Copy link
Contributor

timbrinded commented Mar 7, 2023

  • I'm submitting a ...

    • Bug report
    • Feature request
    • Support request
    • Other
  • What is the current behavior and expected behavior?

As we all know, commonJs / ESM compatibility is really annoying. One such issues is when relative imports lack a .js extension. This means when using the typegen facility of polkadotJs, the generated interfaces (Typescript) are all non-esm compatible:

// Auto-generated via `yarn polkadot-types-from-chain`, do not edit
/* eslint-disable */

import "./augment-api-consts";
import "./augment-api-errors";
import "./augment-api-events";
import "./augment-api-query";

I could look to using a bundler to rewrite paths, but it is far far simpler to just fix it source.


This is not going to be fixed by MS: microsoft/TypeScript#49083 (comment)

Now there are workarounds for this, such as providing an experimental flag to node to override, but these are being phased out in newer versions of node - and instead custom loaders are expected to override this behaviour.

Furthermore, these workarounds are unreliable and tend to fail when the dependency graph is large (like with polkadotJs).

  • What is the motivation for changing the behavior?

To allow me to import augmented apis into a custom substrate test framework, which combines them.

  • Please tell us about your environment:

    • Version:

    • Environment:

      • Node.js
      • Browser
      • Other (limited support for other environments)
    • Language:

      • JavaScript
      • TypeScript (include tsc --version)
      • Other
  • TSC - 4.8.4
  • Node - 14/16/18
@jacogr
Copy link
Member

jacogr commented Mar 8, 2023

"As we all know, commonJs / ESM compatibility is really annoying."

That has to be the understatement of the 2020's :)

It is not just a simple fix here actually since the generation itself also affects all the polkadot-js code which uses extensionless imports everywhere. So it actually means adapting all polkadot-js repos to first use .js imports (changing all code, adding the required eslint rules to detect these) and then the generator can add this.

... that is the bad news, the good news is that I have been pondering doing exactly that. (It probably means re-adjusting the export maps in packages as well)

@jacogr
Copy link
Member

jacogr commented Mar 9, 2023

After a PR marathon above (once all are released stuff like "moduleResolution": "nodenext" will probably work on these libs), this generation may now work as expected in the next release version.

At least the specific commented-on generation will now align, cannot vouch everything else - the typegen stuff is hairy so sadly atm it is a bit of a whack-a-mole to find where templates/code should get extensions.

TL;DR Could work, if not, specifics to "where there are holes" would cover the rest.

@montogeek
Copy link

Related to this, after the PR of this issue #5440 merged, I am not longer unable to generate types.

Getting the same error about Unknown file extension ".ts".

What would be the correct ts-node command with the .mjs file?

@jacogr
Copy link
Member

jacogr commented Mar 14, 2023

@montogeek Ok, that sounds worrying - could you please log a separate issue with to not cross lines - it makes it easy to miss stuff otherwise when seemingly related stuff are bundled together.

Please do include as much details as possible as well -

  • you repo would be good (if available)
  • the exact commands you are running
  • what the full error message is you are seeing

@timbrinded
Copy link
Contributor Author

Verified this is working correctly in v10.1.3. Cheers! 🎉

@polkadot-js-bot
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue if you think you have a related problem or query.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Apr 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants