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

TypeScript imports broken since use of ProtoScript #202

Closed
zolex opened this issue Sep 23, 2022 · 5 comments
Closed

TypeScript imports broken since use of ProtoScript #202

zolex opened this issue Sep 23, 2022 · 5 comments

Comments

@zolex
Copy link

zolex commented Sep 23, 2022

since version 0.0.61 when ProtoScript is used by twirpscript to generate typescript code, the imports of the messages from the services are broken:

it outputs .pb.ts files but the import statements are .pb.js

generated files:
image

import statements before 0.0.61

image

import statements since 0.0.61

image

@zolex zolex changed the title typescript imports broken since ProtoScript code generation TypeScript imports broken since use of ProtoScript Sep 23, 2022
@tatethurston
Copy link
Owner

@zolex Thanks for reporting this. This change is intentional, and was made specifically to support the TypeScript compiler. The full extension path is expected — it’s required for ES modules which don’t implicitly assume a js extension like commonjs does. The TypeScripts compiler expects js extensions and not ts extensions because the compiler does not manipulate import paths: https://www.typescriptlang.org/docs/handbook/esm-node.html. The TypeScript team may reevaluate this in 4.9.

I suspect you're using something other than the TypeScript compiler, such as webpack and ts-loader, to compile your TypeScript code? TypeStrong/ts-loader#1383.

@andrewbeckman ran into this as well and reported it in #192. Given this rough edge, I'm open to reevaluating this.

@tatethurston
Copy link
Owner

This could look like the following:

  • Continue to use full import paths for JS. This delivers correct ESM for browser and node runtimes.
  • Drop the .js extension for TS. Browser builds like webpack accept ESM without the full extension, so this would only prevent users from running TwirpScript server side as ESM (type: "module" in package.json). I suspect this is an acceptable tradeoff until the TS team better aligns with the rest of the JS ecosystem, and I can expose a config option for the current behavior if anyone requests it.

@zolex
Copy link
Author

zolex commented Sep 23, 2022

That's interesting, thanks for the detailed answer.

Our Frontend team is using gatsby, do you know how to address this issue here?

@zolex zolex closed this as completed Sep 23, 2022
@tatethurston
Copy link
Owner

@zolex I've published https://github.com/tatethurston/TwirpScript/releases/tag/v0.0.64 with the changes I mentioned above. This should "just work" for you -- let me know if you encounter any issues.

@zolex
Copy link
Author

zolex commented Sep 23, 2022

Thanks man, I'll ask our responsible dev on monday to see if it solves the issues.

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

No branches or pull requests

2 participants