Add methodName to rpc methods to allow to determine methods after code minification#857
Open
Szpadel wants to merge 4 commits intoprotobufjs:masterfrom
Open
Add methodName to rpc methods to allow to determine methods after code minification#857Szpadel wants to merge 4 commits intoprotobufjs:masterfrom
Szpadel wants to merge 4 commits intoprotobufjs:masterfrom
Conversation
Szpadel
force-pushed
the
master
branch
7 times, most recently
from
484e36a to
e9bc55b
Compare
Author
|
@dcodeIO any chance for merge? |
This reverts commit ed7e2e7.
Messages with optional fields will now implement their corresponding interfaces (IMessage) fixes protobufjs#834 fixes protobufjs#837
avakhrenev
reviewed
Aug 17, 2017
|
|
||
| greeter.on("data", function(response, method) { | ||
| console.log("data in " + method.name + ":", response.message); | ||
| console.log("data in " + method.nameName + ":", response.message); |
Member
|
Can't you just do: function rpcImpl(method, ...) {
if (method == MyService.prototype.myMethod) { ... }
} |
Author
|
@dcodeIO then you would need rpc transport layer to be dependent on all services. |
|
@dcodeIO This is quite cumbersome when building a transport that works with any proto file. Please consider merging this pr. |
dcodeIO
added a commit
that referenced
this pull request
May 18, 2018
…ice method names when generating static code, see #857
Member
|
Does this solve it? Idea is that defining the |
|
@dcodeIO how to get a full method name (in format of |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In current state when static code is minified, generated method names are replaced by single letter names.
This make almost impossible to implement correctly transport layer for services.
Those changes adds new property to methods:
methodName, that contains string to original method name.After this change you should determine called method using
methodNameinstead ofname, but old behavior is still preserved.Also this commit create new exported type:
RpcServiceMethodto allow to access new property in type safe way.Runtime generated code was also extended with generated code.
Although methodName is appended in every runtime rpc call - I didn't found a way how to make it one time action in method generation.
Test case was also extended to make sure that
methodNamewill containe the same value thatname.Bonus:
I fixed Travis build issue for you :-) I don't like to have that red X in my PR ;-)