Skip to content
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

Update web protogen to use protobuf-ts #73

Closed

Conversation

DTCurrie
Copy link
Member

We are having a lot of issues with using the generated web proto files in RDK mainly because the way we currently generate these files uses CommonJs while our applications use ECMAScript modules for file imports. This requires us to use rollup as an intermediary build process to convert the files, which is causing unpredictable changes in our build that then cause issues in the RDK CI.

For example, the code in this PR has been generated by three different users on three different systems, and all of them produce different file name hashes for the built artifacts than the CI does, blocking the change from being able to merge: viamrobotics/rdk#1478

Since RDK just copies the files from API, we need to update how we generate the files here to use more modern web based tools. protobuf-ts will let us generate TS and JS files that are much easier to work with and are more syntactically similar to modern web libraries.

This would allow us to remove the rollup step in RDK and further productionize our build process there. We have already done this change in the app repo and the resulting code was much easier to work with. It also significantly decreases the size of our code base.

@DTCurrie DTCurrie self-assigned this Oct 12, 2022
@DTCurrie DTCurrie added the enhancement New feature or request label Oct 12, 2022
@DTCurrie
Copy link
Member Author

Closing this for now, since we will need to have a PR ready in RDK to handle these first. Going to reopen with a new gen/ts directory for the new builds to test with, then update RDK to use that, and finally use protobuf-ts to handle javascript generation in gen/js.

@DTCurrie DTCurrie closed this Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants