[TS] Fix relative import paths of generated TypeScript code (#8880)

* Refactor logic that generates import paths in AddImport

* Add new tests to validate relative import path fix

* Generate goldens

* Generate example code

* Format TS generator file

* Revert "Format TS generator file"

This reverts commit 0f0b24aee9.

* Fix merge conflicts

---------

Co-authored-by: Björn Harrtell <bjornharrtell@users.noreply.github.com>

goldens/ts/flatbuffers/goldens/universe.ts

@@ -4,7 +4,7 @@
 
 import * as flatbuffers from 'flatbuffers';
 
-import { Galaxy } from '../../flatbuffers/goldens/galaxy.js';
+import { Galaxy } from './galaxy.js';
 
 
 export class Universe {

src/idl_gen_ts.cpp

@@ -324,9 +324,10 @@ class TsGenerator : public BaseGenerator {
         export_counter++;
       }
 
-      if (export_counter > 0)
+      if (export_counter > 0) {
         parser_.opts.file_saver->SaveFile(it.second.filepath.c_str(), code,
                                           false);
+      }
     }
   }
 
@@ -647,7 +648,8 @@ class TsGenerator : public BaseGenerator {
   }
 
   void GenStructArgs(import_set& imports, const StructDef& struct_def,
-                     std::string* arguments, const std::string& nameprefix) {
+                     const Definition& owner, std::string* arguments,
+                     const std::string& nameprefix) {
     for (auto it = struct_def.fields.vec.begin();
          it != struct_def.fields.vec.end(); ++it) {
       auto& field = **it;
@@ -655,11 +657,11 @@ class TsGenerator : public BaseGenerator {
         // Generate arguments for a struct inside a struct. To ensure names
         // don't clash, and to make it obvious these arguments are constructing
         // a nested struct, prefix the name with the field name.
-        GenStructArgs(imports, *field.value.type.struct_def, arguments,
+        GenStructArgs(imports, *field.value.type.struct_def, owner, arguments,
                       nameprefix + field.name + "_");
       } else {
         *arguments += ", " + nameprefix + field.name + ": " +
-                      GenTypeName(imports, field, field.value.type, true,
+                      GenTypeName(imports, owner, field.value.type, true,
                                   field.IsOptional());
       }
     }
@@ -921,6 +923,48 @@ class TsGenerator : public BaseGenerator {
     return symbols_expression;
   }
 
+  std::vector<std::string> PathComponents(const std::string& path) const {
+    std::vector<std::string> components;
+    size_t start = 0;
+    while (start < path.size()) {
+      auto end = path.find(kPathSeparator, start);
+      if (end == std::string::npos) end = path.size();
+      if (end > start) {
+        components.emplace_back(path.substr(start, end - start));
+      }
+      if (end == path.size()) break;
+      start = end + 1;
+    }
+    return components;
+  }
+
+  std::string RelativeDirectory(const std::vector<std::string>& from,
+                                const std::vector<std::string>& to) const {
+    size_t common = 0;
+    while (common < from.size() && common < to.size() &&
+           from[common] == to[common]) {
+      ++common;
+    }
+
+    std::string rel;
+    const size_t ups = from.size() - common;
+    if (ups == 0) {
+      rel = ".";
+    } else {
+      for (size_t i = 0; i < ups; ++i) {
+        if (!rel.empty()) rel += kPathSeparator;
+        rel += "..";
+      }
+    }
+
+    for (size_t i = common; i < to.size(); ++i) {
+      if (!rel.empty()) rel += kPathSeparator;
+      rel += to[i];
+    }
+
+    return rel;
+  }
+
   template <typename DefinitionT>
   ImportDefinition AddImport(import_set& imports, const Definition& dependent,
                              const DefinitionT& dependency) {
@@ -948,26 +992,32 @@ class TsGenerator : public BaseGenerator {
     const std::string symbols_expression = GenSymbolExpression(
         dependency, has_name_clash, import_name, name, object_name);
 
-    std::string bare_file_path;
-    std::string rel_file_path;
-    if (dependent.defined_namespace) {
-      const auto& dep_comps = dependent.defined_namespace->components;
-      for (size_t i = 0; i < dep_comps.size(); i++) {
-        rel_file_path += i == 0 ? ".." : (kPathSeparator + std::string(".."));
-      }
-      if (dep_comps.size() == 0) {
-        rel_file_path += ".";
-      }
-    } else {
-      rel_file_path += "..";
-    }
+    const Namespace* dependent_ns = dependent.defined_namespace
+                                        ? dependent.defined_namespace
+                                        : parser_.empty_namespace_;
+    const Namespace* dependency_ns = dependency.defined_namespace
+                                         ? dependency.defined_namespace
+                                         : parser_.empty_namespace_;
 
-    bare_file_path +=
-        kPathSeparator +
-        namer_.Directories(dependency.defined_namespace->components,
-                           SkipDir::OutputPath) +
+    const std::string dependent_dirs =
+        namer_.Directories(*dependent_ns, SkipDir::OutputPath);
+    const std::string dependency_dirs =
+        namer_.Directories(*dependency_ns, SkipDir::OutputPath);
+
+    const auto dependent_components = PathComponents(dependent_dirs);
+    const auto dependency_components = PathComponents(dependency_dirs);
+
+    std::string rel_dir =
+        RelativeDirectory(dependent_components, dependency_components);
+    if (rel_dir.empty()) rel_dir = ".";
+    if (!rel_dir.empty()) rel_dir += kPathSeparator;
+
+    std::string rel_file_path =
+        rel_dir + namer_.File(dependency, SkipFile::SuffixAndExtension);
+
+    std::string bare_file_path =
+        kPathSeparator + dependency_dirs +
         namer_.File(dependency, SkipFile::SuffixAndExtension);
-    rel_file_path += bare_file_path;
 
     ImportDefinition import;
     import.name = name;
@@ -1172,9 +1222,10 @@ class TsGenerator : public BaseGenerator {
 
   std::string GenNullCheckConditional(const std::string& nullCheckVar,
                                       const std::string& trueVal,
-                                      const std::string& falseVal) {
+                                      const std::string& falseVal = "") {
+    std::string false_val = falseVal.empty() ? null_keyword_ : falseVal;
     return "(" + nullCheckVar + " !== " + null_keyword_ + " ? " + trueVal +
-           " : " + falseVal + ")";
+           " : " + false_val + ")";
   }
 
   std::string GenStructMemberValueTS(const StructDef& struct_def,
@@ -1632,11 +1683,12 @@ class TsGenerator : public BaseGenerator {
     GenDocComment(struct_def.doc_comment, code_ptr);
     code += "export class ";
     code += object_name;
-    if (parser.opts.generate_object_based_api)
+    if (parser.opts.generate_object_based_api) {
       code += " implements flatbuffers.IUnpackableObject<" + object_api_name +
               "> {\n";
-    else
+    } else {
       code += " {\n";
+    }
     code += "  bb: flatbuffers.ByteBuffer|" + null_keyword_ + " = " +
             null_keyword_ + ";\n";
     code += "  bb_pos = 0;\n";
@@ -2054,7 +2106,7 @@ class TsGenerator : public BaseGenerator {
     // Emit a factory constructor
     if (struct_def.fixed) {
       std::string arguments;
-      GenStructArgs(imports, struct_def, &arguments, "");
+      GenStructArgs(imports, struct_def, struct_def, &arguments, "");
       GenDocComment(code_ptr);
 
       code += "static create" + GetPrefixedName(struct_def) +

tests/ts/JavaScriptRelativeImportPathTest.js

@@ -0,0 +1,28 @@
+import { readFileSync } from "node:fs";
+import { fileURLToPath } from "node:url";
+import { dirname, resolve } from "node:path";
+
+const here = dirname(fileURLToPath(import.meta.url));
+const headerPath = resolve(here, "relative_imports/transit/three/header.ts");
+
+const contents = readFileSync(headerPath, "utf8");
+
+const expectedImports = [
+  "from '../one/info.js';",
+  "from '../two/identity.js';",
+];
+
+for (const expected of expectedImports) {
+  if (!contents.includes(expected)) {
+    throw new Error(`Missing relative import "${expected}" in ${headerPath}`);
+  }
+}
+
+const forbidden = "../transit/";
+if (contents.includes(forbidden)) {
+  throw new Error(
+    `Found unexpected namespace segment in import path within ${headerPath}`
+  );
+}
+
+console.log("JavaScriptRelativeImportPathTest: OK");

tests/ts/TypeScriptTest.py

@@ -199,6 +199,20 @@ flatc(
 flatc(options=["--ts"], schema="../long_namespace.fbs")
 flatc(options=["--ts"], schema="../longer_namespace.fbs")
 
+
+flatc(
+    options=[
+        "--ts",
+        "--reflect-names",
+        "--gen-name-strings",
+        "--gen-object-api",
+        "--ts-entry-points",
+        "--ts-flat-files",
+    ],
+    schema="relative_imports/relative_imports.fbs",
+    prefix="relative_imports",
+)
+
 print("Running TypeScript Compiler...")
 check_call(["tsc"])
 print(
@@ -215,6 +229,7 @@ check_call(NODE_CMD + ["JavaScriptUnionVectorTest"])
 check_call(NODE_CMD + ["JavaScriptFlexBuffersTest"])
 check_call(NODE_CMD + ["JavaScriptComplexArraysTest"])
 check_call(NODE_CMD + ["JavaScriptUnionUnderlyingTypeTest"])
+check_call(NODE_CMD + ["JavaScriptRelativeImportPathTest"])
 check_call(NODE_CMD + ["JavaScriptUndefinedForOptionals"])
 
 print("Running old v1 TypeScript Tests...")

tests/ts/my-game/example/any-ambiguous-aliases.ts

@@ -2,7 +2,7 @@
 
 /* eslint-disable @typescript-eslint/no-unused-vars, @typescript-eslint/no-explicit-any, @typescript-eslint/no-non-null-assertion */
 
-import { Monster, MonsterT } from '../../my-game/example/monster.js';
+import { Monster, MonsterT } from './monster.js';
 
 
 export enum AnyAmbiguousAliases {

tests/ts/my-game/example/any-unique-aliases.ts

@@ -2,9 +2,9 @@
 
 /* eslint-disable @typescript-eslint/no-unused-vars, @typescript-eslint/no-explicit-any, @typescript-eslint/no-non-null-assertion */
 
-import { Monster as MyGame_Example2_Monster, MonsterT as MyGame_Example2_MonsterT } from '../../my-game/example2/monster.js';
-import { Monster, MonsterT } from '../../my-game/example/monster.js';
-import { TestSimpleTableWithEnum, TestSimpleTableWithEnumT } from '../../my-game/example/test-simple-table-with-enum.js';
+import { Monster as MyGame_Example2_Monster, MonsterT as MyGame_Example2_MonsterT } from '../example2/monster.js';
+import { Monster, MonsterT } from './monster.js';
+import { TestSimpleTableWithEnum, TestSimpleTableWithEnumT } from './test-simple-table-with-enum.js';
 
 
 export enum AnyUniqueAliases {

tests/ts/my-game/example/any.ts

@@ -2,9 +2,9 @@
 
 /* eslint-disable @typescript-eslint/no-unused-vars, @typescript-eslint/no-explicit-any, @typescript-eslint/no-non-null-assertion */
 
-import { Monster as MyGame_Example2_Monster, MonsterT as MyGame_Example2_MonsterT } from '../../my-game/example2/monster.js';
-import { Monster, MonsterT } from '../../my-game/example/monster.js';
-import { TestSimpleTableWithEnum, TestSimpleTableWithEnumT } from '../../my-game/example/test-simple-table-with-enum.js';
+import { Monster as MyGame_Example2_Monster, MonsterT as MyGame_Example2_MonsterT } from '../example2/monster.js';
+import { Monster, MonsterT } from './monster.js';
+import { TestSimpleTableWithEnum, TestSimpleTableWithEnumT } from './test-simple-table-with-enum.js';
 
 
 export enum Any {

tests/ts/my-game/example/monster.ts

@@ -4,19 +4,19 @@
 
 import * as flatbuffers from 'flatbuffers';
 
-import { Monster as MyGame_Example2_Monster, MonsterT as MyGame_Example2_MonsterT } from '../../my-game/example2/monster.js';
-import { Ability, AbilityT } from '../../my-game/example/ability.js';
-import { Any, unionToAny, unionListToAny } from '../../my-game/example/any.js';
-import { AnyAmbiguousAliases, unionToAnyAmbiguousAliases, unionListToAnyAmbiguousAliases } from '../../my-game/example/any-ambiguous-aliases.js';
-import { AnyUniqueAliases, unionToAnyUniqueAliases, unionListToAnyUniqueAliases } from '../../my-game/example/any-unique-aliases.js';
-import { Color } from '../../my-game/example/color.js';
-import { Race } from '../../my-game/example/race.js';
-import { Referrable, ReferrableT } from '../../my-game/example/referrable.js';
-import { Stat, StatT } from '../../my-game/example/stat.js';
-import { Test, TestT } from '../../my-game/example/test.js';
-import { TestSimpleTableWithEnum, TestSimpleTableWithEnumT } from '../../my-game/example/test-simple-table-with-enum.js';
-import { Vec3, Vec3T } from '../../my-game/example/vec3.js';
-import { InParentNamespace, InParentNamespaceT } from '../../my-game/in-parent-namespace.js';
+import { Monster as MyGame_Example2_Monster, MonsterT as MyGame_Example2_MonsterT } from '../example2/monster.js';
+import { Ability, AbilityT } from './ability.js';
+import { Any, unionToAny, unionListToAny } from './any.js';
+import { AnyAmbiguousAliases, unionToAnyAmbiguousAliases, unionListToAnyAmbiguousAliases } from './any-ambiguous-aliases.js';
+import { AnyUniqueAliases, unionToAnyUniqueAliases, unionListToAnyUniqueAliases } from './any-unique-aliases.js';
+import { Color } from './color.js';
+import { Race } from './race.js';
+import { Referrable, ReferrableT } from './referrable.js';
+import { Stat, StatT } from './stat.js';
+import { Test, TestT } from './test.js';
+import { TestSimpleTableWithEnum, TestSimpleTableWithEnumT } from './test-simple-table-with-enum.js';
+import { Vec3, Vec3T } from './vec3.js';
+import { InParentNamespace, InParentNamespaceT } from '../in-parent-namespace.js';
 
 
 /**

tests/ts/my-game/example/struct-of-structs-of-structs.ts

@@ -4,7 +4,7 @@
 
 import * as flatbuffers from 'flatbuffers';
 
-import { StructOfStructs, StructOfStructsT } from '../../my-game/example/struct-of-structs.js';
+import { StructOfStructs, StructOfStructsT } from './struct-of-structs.js';
 
 
 export class StructOfStructsOfStructs implements flatbuffers.IUnpackableObject<StructOfStructsOfStructsT> {

tests/ts/my-game/example/struct-of-structs.ts

@@ -4,8 +4,8 @@
 
 import * as flatbuffers from 'flatbuffers';
 
-import { Ability, AbilityT } from '../../my-game/example/ability.js';
-import { Test, TestT } from '../../my-game/example/test.js';
+import { Ability, AbilityT } from './ability.js';
+import { Test, TestT } from './test.js';
 
 
 export class StructOfStructs implements flatbuffers.IUnpackableObject<StructOfStructsT> {

tests/ts/my-game/example/test-simple-table-with-enum.ts

@@ -4,7 +4,7 @@
 
 import * as flatbuffers from 'flatbuffers';
 
-import { Color } from '../../my-game/example/color.js';
+import { Color } from './color.js';
 
 
 export class TestSimpleTableWithEnum implements flatbuffers.IUnpackableObject<TestSimpleTableWithEnumT> {

tests/ts/my-game/example/vec3.ts

@@ -4,8 +4,8 @@
 
 import * as flatbuffers from 'flatbuffers';
 
-import { Color } from '../../my-game/example/color.js';
-import { Test, TestT } from '../../my-game/example/test.js';
+import { Color } from './color.js';
+import { Test, TestT } from './test.js';
 
 
 export class Vec3 implements flatbuffers.IUnpackableObject<Vec3T> {

tests/ts/optional-scalars/scalar-stuff.ts

@@ -4,7 +4,7 @@
 
 import * as flatbuffers from 'flatbuffers';
 
-import { OptionalByte } from '../optional-scalars/optional-byte.js';
+import { OptionalByte } from './optional-byte.js';
 
 
 export class ScalarStuff {

tests/ts/relative_imports/relative_imports.fbs

@@ -0,0 +1,20 @@
+namespace Transit.One;
+
+table Info {
+  timestamp:ulong;
+}
+
+namespace Transit.Two;
+
+table Identity {
+  id:uint;
+}
+
+namespace Transit.Three;
+
+table Header {
+  info:Transit.One.Info;
+  id:Transit.Two.Identity;
+}
+
+root_type Header;

tests/ts/tsconfig.json

@@ -1,7 +1,10 @@
 {
   "compilerOptions": {
     "target": "ES2020",
-    "lib": ["ES2020", "DOM"],
+    "lib": [
+      "ES2020",
+      "DOM"
+    ],
     "module": "NodeNext",
     "declaration": true,
     "strict": true
@@ -17,6 +20,7 @@
     "arrays_test_complex/**/*.ts",
     "union_underlying_type_test.ts",
     "long-namespace/**/*.ts",
-    "longer-namespace/**/*.ts"
+    "longer-namespace/**/*.ts",
+    "relative_imports/**/*.ts"
   ]
-}
+}