chaokunyang commented on PR #3394:
URL: https://github.com/apache/fory/pull/3394#issuecomment-4159690218
This PR still have many severe issues:
1. Nested type flattening can generate invalid TypeScript for valid schemas.
the generator emits every nested message/enum/union with its simple name only,
it uses message.name directly for the exported identifier. If two nested class
have same name, this will just fail. nested class should be in style:
```
export class Outer {
// fields...
}
namespace Outer {
class Inner {}
}
```
2. Top-level unions are still not actually supported by the generated
JavaScript API; the PR papers over that by skipping the corresponding IDL
roundtrips. The registration helper explicitly refuses to register unions in
javascript.py
3. The new JavaScript IDL “compatible mode” coverage is a deliberate no-op
workaround, so CI can go green without exercising JavaScript at all for that
mode. The peer immediately exits successfully for compatible mode in
roundtrip.ts and even the normal path hard-codes compatible: false in
roundtrip.ts
Those are obvious issues, I even don't dive into details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]