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

TypeScript: strongly typed lookup function #544

Closed
paralin opened this issue Dec 11, 2016 · 8 comments
Closed

TypeScript: strongly typed lookup function #544

paralin opened this issue Dec 11, 2016 · 8 comments

Comments

@paralin
Copy link

paralin commented Dec 11, 2016

I'm finding myself doing this a lot:

(<any>tree.lookup('mynamespace.MyMessage')).encode...

The reason is that lookup returns a ReflectionObject which I cannot directly cast to any of its children (I can cast a Type -> ReflectionObject but not the other way around).

I'm proposing doing something like:

tree.lookupService('mynamespace.MyService'); // ok -> ProtoBuf.Service
tree.lookupType('mynamespace.MyService'); // throws -> MyService is not a Type

Strongly typed with exceptions if you attempt to lookup the wrong type (or just return null)

@dcodeIO
Copy link
Member

dcodeIO commented Dec 11, 2016

Hmm, the following is passing in the test file:

protobuf.Class.create(root.lookup("Hello") as protobuf.Type, Hello);

@paralin
Copy link
Author

paralin commented Dec 11, 2016

It's possible, but messy. And not particularly safe (in the case that it isn't in fact a Type)

@bb
Copy link

bb commented Dec 12, 2016

The changeset 01365ba helps specifically for Type and Service, but if I understand @paralin correctly, he was looking for a type declaration like

lookup<T>(path: (string|string[]), parentAlreadyChecked?: boolean): T extends (ReflectionObject|Type);

(didn't check that, just trying to generalize the type definition right here in the comment editor)

This would enable him writing

tree.lookup<MyMessage>('mynamespace.MyMessage').encode...

@paralin
Copy link
Author

paralin commented Dec 12, 2016

Actually @bb that's not what I meant, however what you have posted is quite nice and I'd love it if that was in place.

@dcodeIO
Copy link
Member

dcodeIO commented Dec 12, 2016

I'd favor any solution that doesn't require additional code (like it is now with lookupType etc.), and just works on a definition basis. I added the additional functions as there doesn't seem to be a clean way, but if there is, I'll gladly change it.

@xujihui1985
Copy link

Hi @dcodeIO, I tested the following on node v6.9.1, and seems nodejs doesn't happy with that https://github.com/dcodeIO/protobuf.js/blob/master/types/test.ts#L20

it throws error TypeError: Cannot assign to read only property 'prototype' of function on
https://github.com/dcodeIO/protobuf.js/blob/master/src/class.js#L44 , because property prototype on constructor is not writeable while the clazz is an Class

@dcodeIO
Copy link
Member

dcodeIO commented Dec 14, 2016

Try "typescript": "^2.2.0-dev.20161214",

@dcodeIO
Copy link
Member

dcodeIO commented Dec 15, 2016

lookupType and lookupService are now generated programmatically, which makes me feel a lot better about this. Considering this issue fixed for now.

@dcodeIO dcodeIO closed this as completed Dec 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants