This is an automated email from the ASF dual-hosted git repository.
chaokunyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/fory.git
The following commit(s) were added to refs/heads/main by this push:
new bffa8f198 fix(javascript): honor declared element types for root
container registration (#4013)
bffa8f198 is described below
commit bffa8f198a734aa5ebd2420c044d7cb5d06ddbd7
Author: Ayush Kumar <[email protected]>
AuthorDate: Mon Sep 7 09:00:21 2026 +0530
fix(javascript): honor declared element types for root container
registration (#4013)
## What was the error
Registering a root container with declared element types, e.g.
`fory.register(Type.list(Type.float32()))`, silently returned the
internal any-typed container serializer. The declared generics were
discarded: elements were written with dynamic dispatch instead of the
declared type, so declared `float32`/`int64` semantics were lost at the
root while the same declaration worked as a struct field.
## What this PR fixes
A root `Type.list`/`Type.set`/`Type.map` with declared element types now
gets a dedicated generated serializer, bound to the returned root
serialize/deserialize pair. It is kept out of the type-id keyed registry
so dynamic container dispatch stays untouched. Regression tests cover
root list, set, and map plus the dynamic-dispatch guard.
---------
Co-authored-by: Claude Fable 5 <[email protected]>
Co-authored-by: chaokunyang <[email protected]>
---
javascript/packages/core/lib/fory.ts | 17 +++++++----
javascript/packages/core/lib/gen/index.ts | 21 +++++++++----
javascript/packages/core/lib/gen/map.ts | 6 ++--
javascript/packages/core/lib/typeInfo.ts | 26 ++++++++++++++++
javascript/test/array.test.ts | 50 +++++++++++++++++++++++++++++++
javascript/test/map.test.ts | 40 ++++++++++++++++++++++++-
6 files changed, 146 insertions(+), 14 deletions(-)
diff --git a/javascript/packages/core/lib/fory.ts
b/javascript/packages/core/lib/fory.ts
index 8eff8df04..04cd1fd6e 100644
--- a/javascript/packages/core/lib/fory.ts
+++ b/javascript/packages/core/lib/fory.ts
@@ -27,7 +27,7 @@ import {
TypeId,
CustomSerializer,
} from "./type";
-import { InputType, ResultType, TypeInfo } from "./typeInfo";
+import { containerDeclaresElementTypes, InputType, ResultType, TypeInfo } from
"./typeInfo";
import { Gen } from "./gen";
import { PlatformBuffer } from "./platformBuffer";
import { ReadContext, WriteContext } from "./context";
@@ -163,7 +163,12 @@ export default class Fory {
serializer = new Gen(this.typeResolver, {
customSerializer,
}).generateSerializer(typeInfo);
- this.typeResolver.registerSerializer(typeInfo, serializer);
+ if (!containerDeclaresElementTypes(typeInfo)) {
+ // A declared-element container serializer is bound to this
+ // registration only; publishing it under the bare container type id
+ // would replace the dynamic container serializer.
+ this.typeResolver.registerSerializer(typeInfo, serializer);
+ }
}
return {
serializer,
@@ -223,9 +228,11 @@ export default class Fory {
}
const readContext = this.readContext;
const reader = readContext.reader;
- const rootSerializer = TypeId.polymorphicType(serializer.getTypeId())
- ? serializer
- : this.anySerializer;
+ const rootSerializer =
+ TypeId.polymorphicType(serializer.getTypeId()) ||
+ containerDeclaresElementTypes(serializer.getTypeInfo())
+ ? serializer
+ : this.anySerializer;
const rootHeader = ConfigFlags.isCrossLanguageFlag;
rootDeserializer = (bytes: Uint8Array) => {
readContext.reset(bytes);
diff --git a/javascript/packages/core/lib/gen/index.ts
b/javascript/packages/core/lib/gen/index.ts
index 9e6244672..9b9a4f841 100644
--- a/javascript/packages/core/lib/gen/index.ts
+++ b/javascript/packages/core/lib/gen/index.ts
@@ -18,7 +18,7 @@
*/
import { TypeId, Serializer } from "../type";
-import { TypeInfo } from "../typeInfo";
+import { containerDeclaresElementTypes, TypeInfo } from "../typeInfo";
import { CodegenRegistry } from "./router";
import { CodecBuilder } from "./builder";
import { Scope } from "./scope";
@@ -139,11 +139,14 @@ export class Gen {
this.traversalContainer(x);
});
this.register(typeInfo, this.generate(typeInfo));
- } else if (!this.isRegistered(typeInfo) &&
TypeId.structType(typeInfo.typeId)) {
- // Forward reference to a struct type not yet fully defined — register
a
- // placeholder so that serializer factories can capture the object
- // reference. The placeholder will be filled in via Object.assign
- // when the real serializer is generated later.
+ } else if (
+ !this.isRegistered(typeInfo) &&
+ (TypeId.structType(typeInfo.typeId) || TypeId.extType(typeInfo.typeId))
+ ) {
+ // Forward reference to a struct or ext type not yet fully defined —
+ // register a placeholder so that serializer factories can capture the
+ // object reference. The placeholder will be filled in via
+ // Object.assign when the real serializer is generated later.
this.register(typeInfo);
} else if (TypeId.enumType(typeInfo.typeId) &&
!this.isRegistered(typeInfo)) {
this.register(typeInfo, this.generate(typeInfo));
@@ -175,6 +178,12 @@ export class Gen {
generateSerializer(typeInfo: TypeInfo) {
this.traversalContainer(typeInfo);
+ if (containerDeclaresElementTypes(typeInfo)) {
+ // The type-id keyed registry only holds the dynamic container
+ // serializer; a container with declared element types gets a dedicated
+ // serializer for this registration.
+ return this.generate(typeInfo);
+ }
const serializer = this.typeResolver.getSerializerByTypeInfo(typeInfo);
if (serializer?._initialized) {
return serializer;
diff --git a/javascript/packages/core/lib/gen/map.ts
b/javascript/packages/core/lib/gen/map.ts
index d9fcdebf3..4624d4587 100644
--- a/javascript/packages/core/lib/gen/map.ts
+++ b/javascript/packages/core/lib/gen/map.ts
@@ -397,8 +397,10 @@ export class MapSerializerGenerator extends
BaseSerializerGenerator {
}
private useDeclaredType(typeInfo: TypeInfo) {
- const readWriteTypeInfo =
- this.builder.resolver.getSerializerByTypeInfo(typeInfo)?.getTypeInfo()
?? typeInfo;
+ const serializer = this.builder.resolver.getSerializerByTypeInfo(typeInfo);
+ // Forward registrations expose a placeholder whose metadata is
unavailable until the codec
+ // is registered. Keep the declared schema until that serializer is
initialized.
+ const readWriteTypeInfo = serializer?._initialized ?
serializer.getTypeInfo() : typeInfo;
// Evolving structs need per-chunk TypeInfo so a compatible reader can
discard a removed map
// field. A fixed-schema serializer deliberately keeps the declared form:
evolving=false is its
// same-schema size and speed opt-out, even when the field declaration is
only a placeholder.
diff --git a/javascript/packages/core/lib/typeInfo.ts
b/javascript/packages/core/lib/typeInfo.ts
index 95ff4a0a4..a92f438c5 100644
--- a/javascript/packages/core/lib/typeInfo.ts
+++ b/javascript/packages/core/lib/typeInfo.ts
@@ -26,6 +26,32 @@ import { Decimal } from "./types/decimal";
const targetFields = new WeakMap<new () => any, { [key: string]: TypeInfo }>();
export const MAX_FIELD_ID = (1 << 29) - 1;
+/**
+ * Whether this container TypeInfo declares concrete element types instead of
+ * dynamic `any` elements. Such a TypeInfo is a usage schema for one
+ * registration: it needs its own generated serializer and must not replace
+ * the dynamic container serializer in the type-id keyed registry.
+ */
+export function containerDeclaresElementTypes(typeInfo: TypeInfo): boolean {
+ if (typeInfo.typeId === TypeId.LIST) {
+ const inner = typeInfo.options?.inner;
+ return inner !== undefined && inner.typeId !== TypeId.UNKNOWN;
+ }
+ if (typeInfo.typeId === TypeId.SET) {
+ const inner = typeInfo.options?.key;
+ return inner !== undefined && inner.typeId !== TypeId.UNKNOWN;
+ }
+ if (typeInfo.typeId === TypeId.MAP) {
+ const key = typeInfo.options?.key;
+ const value = typeInfo.options?.value;
+ return (
+ (key !== undefined && key.typeId !== TypeId.UNKNOWN) ||
+ (value !== undefined && value.typeId !== TypeId.UNKNOWN)
+ );
+ }
+ return false;
+}
+
export function checkFieldId(fieldId: number) {
if (Number.isFinite(fieldId) && fieldId < 0) {
throw new Error("field id must be non-negative");
diff --git a/javascript/test/array.test.ts b/javascript/test/array.test.ts
index 989bbf7f8..9073fb11a 100644
--- a/javascript/test/array.test.ts
+++ b/javascript/test/array.test.ts
@@ -77,6 +77,56 @@ describe("array", () => {
expect(deserialize(serialize({ c: [o, o] }))).toEqual({ c: [o, o] });
});
+ test("should root list use declared element type", () => {
+ // A root Type.list(...) registration previously fell back to the internal
+ // any-typed list serializer, silently discarding declared element types:
+ // declared float32 must narrow, while dynamic dispatch keeps float64.
+ const fory = new Fory({ compatible: false });
+ const { serialize, deserialize } =
fory.register(Type.list(Type.float32()));
+ expect(deserialize(serialize([0.1]))).toEqual([Math.fround(0.1)]);
+
+ // The dynamic list serializer must stay untouched by the registration.
+ expect(fory.deserialize(fory.serialize([0.1, "a"]))).toEqual([0.1, "a"]);
+ });
+
+ test("should root set use declared element type", () => {
+ const fory = new Fory({ compatible: false });
+ const { serialize, deserialize } = fory.register(Type.set(Type.float32()));
+ expect(deserialize(serialize(new Set([0.1])))).toEqual(new
Set([Math.fround(0.1)]));
+ expect(fory.deserialize(fory.serialize(new Set([0.1, "a"])))).toEqual(new
Set([0.1, "a"]));
+ });
+
+ test("should root container registered before its ext codec work", () => {
+ // Registration order is free before the first root operation: the
+ // container's forward ext placeholder must be filled when the extension
+ // codec registers later, so the generated serializer binds to the
+ // completed codec instead of capturing undefined.
+ class ListedExtension {
+ constructor(public id = 0) {}
+ }
+ Type.ext(921)(ListedExtension);
+ const extCodec = {
+ write(context: any, value: ListedExtension) {
+ context.writeUint8(value.id);
+ },
+ read(context: any, result: ListedExtension) {
+ result.id = context.readUint8();
+ },
+ };
+
+ const listFory = new Fory({ compatible: false });
+ const list = listFory.register(Type.list(Type.ext(921)));
+ listFory.register(ListedExtension, extCodec);
+ const listResult = list.deserialize(list.serialize([new
ListedExtension(7)]));
+ expect(listResult).toEqual([new ListedExtension(7)]);
+
+ const setFory = new Fory({ compatible: false });
+ const set = setFory.register(Type.set(Type.ext(921)));
+ setFory.register(ListedExtension, extCodec);
+ const setResult = set.deserialize(set.serialize(new Set([new
ListedExtension(9)])));
+ expect(setResult).toEqual(new Set([new ListedExtension(9)]));
+ });
+
test("preserves a self-reference in a dynamic list", () => {
const fory = new Fory({ compatible: false, ref: true });
const value: any[] = [];
diff --git a/javascript/test/map.test.ts b/javascript/test/map.test.ts
index 367170aff..025ada82f 100644
--- a/javascript/test/map.test.ts
+++ b/javascript/test/map.test.ts
@@ -17,7 +17,7 @@
* under the License.
*/
-import Fory, { Type } from "../packages/core/index";
+import Fory, { ReadContext, Type, WriteContext } from "../packages/core/index";
import { CodegenRegistry } from "../packages/core/lib/gen/router";
import { BinaryReader } from "../packages/core/lib/reader";
import { ConfigFlags, RefFlags, TypeId } from "../packages/core/lib/type";
@@ -97,6 +97,44 @@ describe("map", () => {
});
});
+ test("should root map use declared key and value types", () => {
+ // A root Type.map(...) registration previously fell back to the internal
+ // any-typed map serializer, silently discarding declared key/value types:
+ // declared float32 must narrow, while dynamic dispatch keeps float64.
+ const fory = new Fory({ compatible: false });
+ const { serialize, deserialize } = fory.register(Type.map(Type.string(),
Type.float32()));
+ expect(deserialize(serialize(new Map([["a", 0.1]])))).toEqual(
+ new Map([["a", Math.fround(0.1)]]),
+ );
+
+ // The dynamic map serializer must stay untouched by the registration.
+ expect(fory.deserialize(fory.serialize(new Map([[1, "x"]])))).toEqual(new
Map([[1, "x"]]));
+ });
+
+ test.each([false, true])("registers map before ext codec (%s)", (compatible)
=> {
+ class MapExtension {
+ constructor(public id = 0) {}
+ }
+ Type.ext(922)(MapExtension);
+
+ const fory = new Fory({ compatible });
+ const keys = fory.register(Type.map(Type.ext(922), Type.string()));
+ const values = fory.register(Type.map(Type.string(), Type.ext(922)));
+ fory.register(MapExtension, {
+ write(context: WriteContext, value: MapExtension) {
+ context.writeUint8(value.id);
+ },
+ read(context: ReadContext, result: MapExtension) {
+ result.id = context.readUint8();
+ },
+ });
+
+ const keyInput = new Map([[new MapExtension(7), "key"]]);
+ const valueInput = new Map([["value", new MapExtension(9)]]);
+ expect(keys.deserialize(keys.serialize(keyInput))).toEqual(keyInput);
+
expect(values.deserialize(values.serialize(valueInput))).toEqual(valueInput);
+ });
+
test("preserves shared dynamic map entries", () => {
const fory = new Fory({ compatible: false, ref: true });
@Type.struct(301, {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
