tatethurston/TwirpScript

Generating TypeScript files from proto files that import other proto files is leading to js imports

Closed this issue · 18 comments

I have a proto file with the following import line:

import "product/models.proto";

And in the generated .pb.ts file, there's the following line:

import { UUID, UUIDJSON } from "../proto/uuid/models.pb.js";

My twirp config is as follows:

{
  "root": "../rd",
  "exclude": ["^(?!rpc).+"],
  "dest": "src/generated",
  "language": "typescript"
}

Should the generated import statement be importing from "../proto/uuid/models.pb.ts" or even just "../proto/uuid/models.pb" given that the generated file being imported is a ts file not a js file? I am currently running into blocking errors trying to build the generated code with Next.js because of this. I can try and work around those build issues, but it seems like this may genuinely be unintended.

Hey @andrewbeckman the full extension path is expected — it’s required for ES modules which don’t implicitly append 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

What is the error you’re encountering? And what version of TypeScript are you using?

Screen Shot 2022-08-23 at 3 54 30 PM

TypeScript version: 4.5.4

Here's my tsconfig in case it helps

{
  "compilerOptions": {
    "target": "ES2020",
    "lib": ["dom", "dom.iterable", "esnext"],
    "allowJs": true,
    "skipLibCheck": true,
    "strict": false,
    "forceConsistentCasingInFileNames": true,
    "noEmit": true,
    "esModuleInterop": true,
    "module": "esnext",
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "isolatedModules": true,
    "jsx": "preserve",
    "incremental": true,
    "baseUrl": "src",
    "strictNullChecks": true,
    "strictFunctionTypes": false,
    "downlevelIteration": true,
    "noUnusedLocals": true /* Report errors on unused locals. */,
    "noUnusedParameters": true /* Report errors on unused parameters. */
  },
  "include": [
    "src/**/*.ts",
    "src/**/*.tsx",
    "next-env.d.ts",
    "types/*.d.ts",
    "jest.setup.ts"
  ],
  "exclude": ["node_modules", "src/generated"]
}

Thanks @andrewbeckman, I'll look into this. There is likely a rough edge here, sorry about that. NextJS builds commonjs for the server build and esm for the client, so it sits right in the middle of a difficult transitional period for the JS ecosystem.

In the meantime, you may be able to resolve this by updating your tsconfig compilerOptions.moduleResolution to nodenext/ node16.

Okay thanks @tatethurston. I had already tried updating the moduleResolution to both those values and unfortunately that introduced more issues.

Cool, worth a try. Yeah, any project imports that aren't valid ESM would fail under the nodenext setting.

@andrewbeckman Could you update to TS 4.7 (4.7.4 is the latest version) and let me know if that resolves the issue for you?

https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#ecmascript-module-support-in-node-js

@andrewbeckman Could you update to TS 4.7 (4.7.4 is the latest version) and let me know if that resolves the issue for you?

https://devblogs.microsoft.com/typescript/announcing-typescript-4-7/#ecmascript-module-support-in-node-js

I had already tried this as well. Unfortunately, same issue.

Does tsc compile successfully in your project? I may need to look at Next's build process. I just tested the following TypeScript compiler config:

{
  "compilerOptions": {
    "target": "ES2020",
    "module": "esnext",
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "strict": true /
    "skipLibCheck": true 
  }
}

Does tsc compile successfully in your project? I may need to look at Next's build process. I just tested the following TypeScript compiler config:

{
  "compilerOptions": {
    "target": "ES2020",
    "module": "esnext",
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "strict": true /
    "skipLibCheck": true 
  }
}

Yes! tsc compiles fine when I run yarn tsc.

Awesome. I suspect this is an issue with next’s build process. In particular, I suspect they’re using a babel plugin for ts compilation and it doesn’t have the same behavior as the TS compiler.

Alright I chased this down and the issue is a difference between the TypeScript compiler and webpack: TypeStrong/ts-loader#1383

Webpack introduced extensionAlias to solve this problem: https://webpack.js.org/configuration/resolve/#resolveextensionalias

Here's an example of wiring this up: https://github.com/tatethurston/TwirpScript/pull/195/files#diff-02b18277d8fea71eaebddca674b4e25b357dcc8acb1feebfcc3573d015efb989

It looks like the TypeScript team may reevaluate this in 4.9

LMK if this doesn't resolve the issue for you @andrewbeckman, and 🤞 that this step can be eradicated once TS 4.9 lands.

LMK if this doesn't resolve the issue for you @andrewbeckman, and 🤞 that this step can be eradicated once TS 4.9 lands.

Thanks for hunting this down. I saw this and looked into modifying the webpack config and Next seems to allow it though it discourages it.

Yep: https://nextjs.org/docs/api-reference/next.config.js/custom-webpack-config. I wouldn’t be concerned about risk here. The caution is intended for things that depend on or would interfere with Nextjs’ webpack configuration, which this does not.

module.exports = {
  webpack: (config ) => {
    config.resolve.extensionAlias = {
      ".js": [".ts", ".js"]
    };
    return config;
  },
}

@andrewbeckman I've released https://github.com/tatethurston/TwirpScript/releases/tag/v0.0.64 which removes the need for the extensionAlias workaround we discussed earlier.

@andrewbeckman I've released https://github.com/tatethurston/TwirpScript/releases/tag/v0.0.64 which removes the need for the extensionAlias workaround we discussed earlier.

@tatethurston Thanks for the heads up! We ended up making the switch to https://github.com/timostamm/protobuf-ts as we didn't want to go down the path of customizing our webpack config. Next time I have some time on my hands, will be sure to check out how things have progressed and decide if we should switch back!

Related: TwirpScript now includes a NextJS example: https://github.com/tatethurston/twirpscript/tree/main/examples/nextjs