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.

Sign up for GitHub

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

Jump to bottom

Automatic message registry #131

Open
fenos opened this issue Jul 17, 2021 · 5 comments · May be fixed by #371
Open

Automatic message registry #131

fenos opened this issue Jul 17, 2021 · 5 comments · May be fixed by #371
Labels
enhancement New feature or request

Comments

@fenos
Copy link
Contributor

fenos commented Jul 17, 2021

It will be hugely beneficial if we can create a registry of all proto messages automatically so that we don't need to pass typeRegistry for the Any type which is kind of annoying and hard to maintain.

The protobuf golang implementation does exactly this, whenever a proto generated file is imported it has line at the top of the file, something like:

registry.Register("ProtoMessage", ProtoMessage);

in fact the Any types are always automatically handled as far as the file is imported (and registered).

Definately a great development experience.

We can of course keep the typeRegistry option if we ever need to pass those manually in, otherwise it passes the registry by default
@timostamm timostamm added the enhancement New feature or request label Jul 17, 2021
@timostamm
Copy link
Owner

I've seen this from a different perspective so far: Users have to import to register anyway - so it can just as well be completely explicit. This works out best in C#, where users can easily scan an assembly for messages and register. No funny import side effects, no need to maintain.

I see your point though, and think this is beneficial. It would just need

  • a globalTypeRegistry, exported from @protobuf-ts/runtime,
  • a globalTypeRegistry.push for each export const MyMessage in generated code
  • support in JSON read / write, probably simply by setting the default value for typeRegistry to the global one.

It would also need proper testing, and a documentation update. I'm super constrained at the moment, but if you could PR this I can take care of the docs.

@fenos
Copy link
Contributor Author

fenos commented Jul 22, 2021

@timostamm I'll try to PR this weekend

@sessfeld
Copy link
Contributor

Is this still something that we want? I had to implement that for us, if you'd be interested I could upstream that. In that case, I'd like some pointer on where to put the test for that, I'd guess into test-generated?

@timostamm
Copy link
Owner

Hey Sebastian, I'd love to hear what approach you took. Is it similar to the globalTypeRegistry.push above?

@sessfeld sessfeld linked a pull request Sep 7, 2022 that will close this issue
Open
@sessfeld
Copy link
Contributor

sessfeld commented Sep 7, 2022

Pretty much, I added a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add global type registry sessfeld/protobuf-ts
3 participants
@timostamm @fenos @sessfeld