fix: Struct wrap/unwrap snake case #579

[Celend]

fixes #579

please review :D

[stephenh]

Nice! This lgtm @Celend .

To make sure we don't break this in the future, can you add a struct example to our snake-case test:

https://github.com/stephenh/ts-proto/tree/main/integration/simple-snake

And use the yarn proto2bin/bin2ts commands to generate + check-in code that includes the now-fixed struct logic?

Thanks!

[Celend]

there are two different types of snake case about struct: with/without oneof union, how do I test all cases? @stephenh

create multiple folders in integration?

[stephenh]

@Celend yeah, if we needed to test both snake + w/union and snake + w/o union (afaiu those are the two combinations), then we'd need two separate test directories.

Fwiw if we can reproduce just one of those two in the existing simple-snake test, I'm good with calling that good enough, but also yeah ideally we'd 1) add a struct to the existing simple-snake test, and 2) copy/paste oneof-unions and make it oneof-unions-snake, and then we'd have both combinations covered.

(If that sounds right, I'm kind of skimming/reading quickly, so feel free to use better test names/etc., if I'm not 100% right on which two cases we need to cover.)

[Celend]

@stephenh It can't be done in one integration test because it's control by parameters.txt, so I create a new test.

please take a look, the test is still running and slow😭

If we want test two cases in one integration test, I think we need something like fixture that can use multiple parameters.txt to running one test suite, but also heavier overhead.

[stephenh]

Looks great, thanks @Celend !

[stephenh]

🎉 This PR is included in version 1.115.4 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release
• v1.115.4

Your semantic-release bot 📦🚀