-
Notifications
You must be signed in to change notification settings - Fork 246
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(javascript): Added serialization and serialization for Type Meta Layer #1825
Conversation
javascript/packages/fury/lib/fury.ts
Outdated
@@ -44,6 +48,8 @@ export default class { | |||
this.referenceResolver = new ReferenceResolver(this.binaryReader); | |||
this.classResolver.init(this); | |||
this.anySerializer = new AnySerializer(this); | |||
this.typeMeta = new TypeMeta(BigInt(0),[]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from_bytes
in TypeMeta is a static method, if you want to call it, you should replace by this.typeMeta = TypeMeta
Hi @theweipeng on line 147 , when I do this - ${this.builder.typeMeta.from_bytes(this.builder.reader.ownName())}
+ ${this.builder.reader.buffer(buffer.byteLength)} Please for a hint on what I am missing. |
Why did you remove reading the typeMeta? I am confused |
@theweipeng I was just experimenting . After solving the syntax error, I was trying to figure out a way to pass the tests. |
Ensure that the byte length consumed from the reader matches the length written to the buffer in the writeStmt. In the context of your code, it should be expressed as |
@theweipeng , please have a look at my PR |
Can you add a config key to Config when constructing fury, enum Mode {
SchemaConsistent,
Compatible,
}
new Fury({
mode: Mode
}) And only write/read typeMeta when the mode is Compatible. |
Hello @theweipeng , please is this the right place to add it. https://github.com/apache/fury/blob/main/javascript/packages/fury/lib/fury.ts#L36 |
Yes |
I seem to have arrived at new errors. My I know the default of fury Mode ? |
default mode should be consistenceMode not compatiableMode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What does this PR do?
Related issues
Does this PR introduce any user-facing change?
Benchmark
The TypeScript file Before :
The Result as JavaScript: