TypeScript: drop '.js' extension from '.pb.js' imports (#17)

CHANGELOG.md

@@ -1,5 +1,24 @@
 # Changelog
 
+## v0.0.11
+
+- Revert `Include file extensions in generated file imports` introduced in `v0.0.7` for TypeScript users. Generated TypeScript imports will revert to the following:
+
+```diff
+- import { Foo } from './foo.pb.js';
++ import { Foo } from './foo.pb';
+```
+
+When targeting ESM, the TypeScript compiler expects `.js` extensions and not `.ts` extensions for imports because the compiler does not manipulate import paths: https://www.typescriptlang.org/docs/handbook/esm-node.html.
+
+Including a full extension results in the following TypeScript error:
+
+```
+[tsserver 2691] [E] An import path cannot end with a '.ts' extension.
+```
+
+The TypeScript team's recommendation to use `.js` extensions for `.ts` file imports when targeting ESM causes a number of issues with the broader JavaScript ecosystem. Until this situation is rectified, ProtoScript will not emit ESM compliant extensions for TypeScript. This only impacts TypeScript users who wish to target ESM in Node.JS using the TypeScript compiler, as bundlers are not pedantic about file extensions. If you're impacted by this, please join the discussion in [#202](https://github.com/tatethurston/TwirpScript/issues/202.)
+
 ## v0.0.10
 
 - Change configuration file format. Now, the configuration file is JS instead of JSON. This provides better discoverability and type checking for TypeScript users.

README.md

@@ -71,7 +71,6 @@ If you're looking for an RPC framework, you may be interested in [TwirpScript](h
 
 - Node.js v16 or greater
 - TypeScript v4.7 or greater when using TypeScript
-- [webpack setup](#webpack-setup) when using a compiler other than TypeScript (like webpack)
 
 ## Examples 🚀
 
@@ -321,33 +320,6 @@ ProtoScript's JSON serialization/deserialization implements the [proto3 specific
 
 ProtoScript will serialize JSON keys as `lowerCamelCase` versions of the proto field. Per the proto3 spec, the runtime will accept both `lowerCamelCase` and the original proto field name when deserializing. You can provide the `json_name` field option to specify an alternate key name. When doing so, the runtime will encode JSON messages using the the `json_name` as the key, and will decode JSON messages using the `json_name` if present, otherwise falling back to the `lowerCamelCase` name and finally to the original proto field name.
 
-## Webpack Setup
-
-If you're using a compiler other than TypeScript, such as webpack, this [outstanding issue in TypeScript](https://github.com/microsoft/typescript/issues/37582) adds an additional hurdle.
-
-The short summary of the issue:
-
-- Extensions are required for ESM imports.
-- Extension-less imports have sidestepped this TypeScript issue, but extensions are required for ESM adoption.
-- The TypeScript compiler requires `.js` extensions for imports.
-- Compilers other than TypeScript expect `.ts` extensions for imports.
-
-The TypeScript team is investigating [improving this issue](https://github.com/microsoft/TypeScript/issues/37582) in their [4.9 plan](https://github.com/microsoft/TypeScript/issues/50457)
-
-Webpack users can use [extensionAlias](https://webpack.js.org/configuration/resolve/#resolveextensionalias) to solve this problem:
-
-```diff
-module.exports = {
-+  resolve: {
-+    extensionAlias: {
-+      ".js": [".ts", ".js"],
-+    },
-+  },
-};
-```
-
-For more context, see [TypeScript#37582](https://github.com/microsoft/typescript/issues/37582) and [Webpack#13252](https://github.com/webpack/webpack/issues/13252).
-
 ## Contributing 👫
 
 PR's and issues welcomed! For more guidance check out [CONTRIBUTING.md](https://github.com/tatethurston/protoscript/blob/main/CONTRIBUTING.md)

public.package.json

@@ -1,6 +1,6 @@
 {
   "name": "protoscript",
-  "version": "0.0.10",
+  "version": "0.0.11",
   "description": "A Protobuf runtime and code generation tool for JavaScript and TypeScript",
   "license": "MIT",
   "author": "Tate <[email protected]>",

src/codegen/autogenerate/index.ts

@@ -786,7 +786,7 @@ export function generate(
 
   IMPORT_TRACKER = { ...DEFAULT_IMPORT_TRACKER };
 
-  const ast = processTypes(fileDescriptorProto, identifierTable);
+  const ast = processTypes(fileDescriptorProto, identifierTable, config.isTS);
   const { imports, types } = ast;
   const sourceFile = fileDescriptorProto.getName();
   if (!sourceFile) {

src/codegen/utils.ts

@@ -331,6 +331,10 @@ function stripProtoExtension(protoFileName: string): string {
   return protoFileName.replace(".proto", "");
 }
 
+function getProtobufTSFileNameImport(protoFileName: string): string {
+  return stripProtoExtension(protoFileName) + ".pb";
+}
+
 export function getProtobufTSFileName(protoFileName: string): string {
   return stripProtoExtension(protoFileName) + ".pb.ts";
 }
@@ -561,7 +565,8 @@ function getIdentifierEntryFromTable(
 function getImportForIdentifier(
   identifier: string,
   identifiers: IdentifierTable,
-  fileDescriptorProto: FileDescriptorProto
+  fileDescriptorProto: FileDescriptorProto,
+  isTS: boolean
 ): Import {
   const dep = getIdentifierEntryFromTable(
     identifier,
@@ -573,7 +578,18 @@ function getImportForIdentifier(
 
   const importPath = relative(
     dirname(sourceFile),
-    getProtobufJSFileName(dependencyImportPath)
+    /*
+     * When targetting ESM, 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.
+     *
+     * Including a full extension results in:
+     *
+     * [tsserver 2691] [E] An import path cannot end with a '.ts' extension.
+     *
+     * The TypeScript team's recomendation to use .js extensions for .ts file imports causes a number of issues with the broader JavaScript ecosystem. Until this situation is rectified, do not emit ESM compliant extensions for TypeScript. This only impacts TypeScript users who wish to target ESM in Node.JS. If you're impacted by this, please join the discussion in https://github.com/tatethurston/TwirpScript/issues/202.
+     */
+    isTS
+      ? getProtobufTSFileNameImport(dependencyImportPath)
+      : getProtobufJSFileName(dependencyImportPath)
   );
   const path = getImportPath(importPath);
 
@@ -623,7 +639,8 @@ function isNotBlank<T>(x: T): x is NonNullable<T> {
 
 export function processTypes(
   fileDescriptorProto: FileDescriptorProto,
-  identifierTable: IdentifierTable
+  identifierTable: IdentifierTable,
+  isTS: boolean
 ): ParsedAst {
   const typeFile: ParsedAst = {
     packageName: fileDescriptorProto.getPackage(),
@@ -636,7 +653,8 @@ export function processTypes(
     const _import = getImportForIdentifier(
       identifier,
       identifierTable,
-      fileDescriptorProto
+      fileDescriptorProto,
+      isTS
     );
     const exisitingImport = typeFile.imports.find(
       ({ path }) => path === _import.path