Skip to content

Commit

Permalink
[TS] Fix four bugs with imported types in TypeScript. (#6054)
Browse files Browse the repository at this point in the history 
* [TS] Fix four bugs with imported types in TypeScript.

* When a type had a vector of imported enums:

1) the enum type's file wasn't added to the generated code's list of
imports; and

2) the enum wasn't prefixed with the NS<hash> prefix and wasn't getting
resolved; but

3) non-enum types (ie, "flatbuffers.Offset") were getting the NS<hash>
prefix when they weren't.

* Also, type name prefixes weren't properly attributed with imported
structs in unions because the source definition passed to the typename
prefixing method was for the union, not for the location of the imported
struct.

* clang fmt

* Use of enum_def / struct_def for prefixing types needs to have the files
added to imported files when not generating all types.

* clang fmt
  • Loading branch information
@maxburke
maxburke authored Aug 3, 2020
1 parent a0fb305 commit c30a87d
Showing 1 changed file with 72 additions and 17 deletions.
89 changes: 72 additions & 17 deletions src/idl_gen_js_ts.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class JsTsGenerator : public BaseGenerator {
reexport_map reexports;

std::string enum_code, struct_code, import_code, exports_code, code;
generateEnums(&enum_code, &exports_code, reexports);
generateEnums(&enum_code, &exports_code, reexports, imported_files);
generateStructs(&struct_code, &exports_code, imported_files);
generateImportDependencies(&import_code, imported_files);
generateReexports(&import_code, reexports, imported_files);
Expand Down Expand Up @@ -165,12 +165,15 @@ class JsTsGenerator : public BaseGenerator {

// Generate code for all enums.
void generateEnums(std::string *enum_code_ptr, std::string *exports_code_ptr,
reexport_map &reexports) {
reexport_map &reexports,
imported_fileset &imported_files) {
for (auto it = parser_.enums_.vec.begin(); it != parser_.enums_.vec.end();
++it) {
auto &enum_def = **it;
GenEnum(enum_def, enum_code_ptr, exports_code_ptr, reexports, false);
GenEnum(enum_def, enum_code_ptr, exports_code_ptr, reexports, true);
GenEnum(enum_def, enum_code_ptr, exports_code_ptr, reexports,
imported_files, false);
GenEnum(enum_def, enum_code_ptr, exports_code_ptr, reexports,
imported_files, true);
}
}

Expand Down Expand Up @@ -327,7 +330,7 @@ class JsTsGenerator : public BaseGenerator {
// Generate an enum declaration and an enum string lookup table.
void GenEnum(EnumDef &enum_def, std::string *code_ptr,
std::string *exports_ptr, reexport_map &reexports,
bool reverse) {
imported_fileset &imported_files, bool reverse) {
if (enum_def.generated) return;
if (reverse && lang_.language == IDLOptions::kTs) return; // FIXME.
std::string &code = *code_ptr;
Expand Down Expand Up @@ -385,7 +388,7 @@ class JsTsGenerator : public BaseGenerator {

if (lang_.language == IDLOptions::kTs) {
if (enum_def.is_union) {
code += GenUnionConvFunc(enum_def.underlying_type);
code += GenUnionConvFunc(enum_def.underlying_type, imported_files);
}
if (!ns.empty()) { code += "\n}"; }
}
Expand Down Expand Up @@ -720,7 +723,8 @@ class JsTsGenerator : public BaseGenerator {
return std::string("T") + (UnionHasStringType(union_enum) ? "|string" : "");
}

std::string GenUnionTypeTS(const EnumDef &union_enum) {
std::string GenUnionTypeTS(const EnumDef &union_enum,
imported_fileset &imported_files) {
std::string ret;
std::set<std::string> type_list;

Expand All @@ -733,8 +737,12 @@ class JsTsGenerator : public BaseGenerator {
if (ev.union_type.base_type == BASE_TYPE_STRING) {
type = "string"; // no need to wrap string type in namespace
} else if (ev.union_type.base_type == BASE_TYPE_STRUCT) {
if (!parser_.opts.generate_all) {
imported_files.insert(ev.union_type.struct_def->file);
}

type = GenPrefixedTypeName(WrapInNameSpace(*(ev.union_type.struct_def)),
union_enum.file);
ev.union_type.struct_def->file);
} else {
FLATBUFFERS_ASSERT(false);
}
Expand Down Expand Up @@ -790,11 +798,12 @@ class JsTsGenerator : public BaseGenerator {
return "unionListTo" + enum_def.name;
}

std::string GenUnionConvFunc(const Type &union_type) {
std::string GenUnionConvFunc(const Type &union_type,
imported_fileset &imported_files) {
if (union_type.enum_def) {
const auto &enum_def = *union_type.enum_def;

const auto valid_union_type = GenUnionTypeTS(enum_def);
const auto valid_union_type = GenUnionTypeTS(enum_def, imported_files);
const auto valid_union_type_with_null = valid_union_type + "|null";

auto ret = "\n\nexport function " + GenUnionConvFuncName(enum_def) +
Expand All @@ -803,6 +812,10 @@ class JsTsGenerator : public BaseGenerator {
valid_union_type_with_null +
"\n): " + valid_union_type_with_null + " {\n";

if (!parser_.opts.generate_all) {
imported_files.insert(union_type.enum_def->file);
}

const auto enum_type = GenPrefixedTypeName(
WrapInNameSpace(*(union_type.enum_def)), union_type.enum_def->file);
const auto &union_enum = *(union_type.enum_def);
Expand All @@ -822,7 +835,8 @@ class JsTsGenerator : public BaseGenerator {
ret += "return " + accessor_str + "'') as string;";
} else if (ev.union_type.base_type == BASE_TYPE_STRUCT) {
const auto type = GenPrefixedTypeName(
WrapInNameSpace(*(ev.union_type.struct_def)), union_enum.file);
WrapInNameSpace(*(ev.union_type.struct_def)),
ev.union_type.struct_def->file);
ret += "return " + accessor_str + "new " + type + "())! as " +
type + ";";
} else {
Expand Down Expand Up @@ -960,7 +974,8 @@ class JsTsGenerator : public BaseGenerator {
}

void GenObjApi(const Parser &parser, StructDef &struct_def,
std::string &obj_api_unpack_func, std::string &obj_api_class) {
std::string &obj_api_unpack_func, std::string &obj_api_class,
imported_fileset &imported_files) {
const auto class_name = GetObjApiClassName(struct_def, parser.opts);

std::string unpack_func =
Expand Down Expand Up @@ -1033,6 +1048,10 @@ class JsTsGenerator : public BaseGenerator {
if (IsScalar(field.value.type.base_type) ||
field.value.type.base_type == BASE_TYPE_STRING) {
if (field.value.type.enum_def) {
if (!parser_.opts.generate_all) {
imported_files.insert(field.value.type.enum_def->file);
}

field_type +=
GenPrefixedTypeName(GenTypeName(field.value.type, false, true),
field.value.type.enum_def->file);
Expand Down Expand Up @@ -1141,6 +1160,10 @@ class JsTsGenerator : public BaseGenerator {
}
default: {
if (vectortype.enum_def) {
if (!parser_.opts.generate_all) {
imported_files.insert(vectortype.enum_def->file);
}

field_type +=
GenPrefixedTypeName(GenTypeName(vectortype, false, true),
vectortype.enum_def->file);
Expand All @@ -1166,6 +1189,10 @@ class JsTsGenerator : public BaseGenerator {
}

case BASE_TYPE_UNION: {
if (!parser_.opts.generate_all) {
imported_files.insert(field.value.type.enum_def->file);
}

field_type +=
GenObjApiUnionTypeTS(parser.opts, *(field.value.type.enum_def));

Expand Down Expand Up @@ -1489,6 +1516,12 @@ class JsTsGenerator : public BaseGenerator {
case BASE_TYPE_VECTOR: {
auto vectortype = field.value.type.VectorType();
auto vectortypename = GenTypeName(vectortype, false);

if (vectortype.enum_def) {
vectortypename = GenPrefixedTypeName(vectortypename,
vectortype.enum_def->file);
}

auto inline_size = InlineSize(vectortype);
auto index = GenBBAccess() +
".__vector(this.bb_pos + offset) + index" +
Expand Down Expand Up @@ -1541,6 +1574,10 @@ class JsTsGenerator : public BaseGenerator {
code += prefix + ",optionalEncoding?:any";
} else {
code += prefix;

if (vectortype.enum_def && !parser_.opts.generate_all) {
imported_files.insert(vectortype.enum_def->file);
}
}
code += "):" + vectortypename + "|null {\n";
} else {
Expand Down Expand Up @@ -1643,6 +1680,10 @@ class JsTsGenerator : public BaseGenerator {
if (lang_.language == IDLOptions::kTs) {
std::string type;
if (field.value.type.enum_def) {
if (!parser_.opts.generate_all) {
imported_files.insert(field.value.type.enum_def->file);
}

type = GenPrefixedTypeName(GenTypeName(field.value.type, true),
field.value.type.enum_def->file);
} else {
Expand Down Expand Up @@ -1855,6 +1896,14 @@ class JsTsGenerator : public BaseGenerator {
code += sig_begin + type_old + sig_end + ";\n";
type = type_new + "|Uint8Array";
}
} else {
if (vector_type.enum_def) {
if (!parser_.opts.generate_all) {
imported_files.insert(vector_type.enum_def->file);
}

type = GenPrefixedTypeName(type, vector_type.enum_def->file);
}
}
code += sig_begin + type + sig_end + " {\n";
} else {
Expand Down Expand Up @@ -1994,7 +2043,8 @@ class JsTsGenerator : public BaseGenerator {
if (parser_.opts.generate_object_based_api) {
std::string obj_api_class;
std::string obj_api_unpack_func;
GenObjApi(parser_, struct_def, obj_api_unpack_func, obj_api_class);
GenObjApi(parser_, struct_def, obj_api_unpack_func, obj_api_class,
imported_files);

code += obj_api_unpack_func + "}\n" + obj_api_class;
} else {
Expand All @@ -2005,10 +2055,15 @@ class JsTsGenerator : public BaseGenerator {
}

std::string GetArgType(const FieldDef &field) {
if (field.value.type.enum_def)
return GenPrefixedTypeName(GenTypeName(field.value.type, true),
field.value.type.enum_def->file);
return GenTypeName(field.value.type, true);
auto type_name = GenTypeName(field.value.type, true);

if (field.value.type.enum_def) {
if (IsScalar(field.value.type.base_type)) {
return GenPrefixedTypeName(type_name, field.value.type.enum_def->file);
}
}

return type_name;
}

static std::string GetArgName(const FieldDef &field) {
Expand Down

0 comments on commit c30a87d

Please sign in to comment.