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]

Reply via email to