worker: replace message types string by constants #21537
Conversation
lib/internal/worker.js
Outdated
| ERROR_MESSAGE: 'errorMessage', | ||
| STDIO_PAYLOAD: 'stdioPayload', | ||
| STDIO_WANTS_MORE_DATA: 'stdioWantsMoreData', | ||
| LOAD_SCRIPT: 'loadScript' |
There was a problem hiding this comment.
Changing these strings into numbers may have better performance when comparing message.type. Is it worth to do that ?
There was a problem hiding this comment.
What about our regular sort of constants kLoadScript kStdioWantsMoreData etc?
lib/internal/worker.js
Outdated
| @@ -47,6 +47,15 @@ const kIncrementsPortRef = Symbol('kIncrementsPortRef'); | |||
|
|
|||
| const debug = util.debuglog('worker'); | |||
|
|
|||
| const messageTypes = { | |||
| UP_AND_RUNNING: 'upAndRunning', | |||
| COULD_NOT_SERIALIZEERROR: 'couldNotSerializeError', | |||
There was a problem hiding this comment.
I’d maybe add a _ between SERIALIZE and ERROR ;)
starkwang
force-pushed
the
worker-message-types
branch
from
9f6c5d6 to
91307e5
Compare
lib/internal/worker.js
Outdated
| @@ -47,6 +47,15 @@ const kIncrementsPortRef = Symbol('kIncrementsPortRef'); | |||
|
|
|||
| const debug = util.debuglog('worker'); | |||
|
|
|||
| const messageTypes = { | |||
| UP_AND_RUNNING: 'kUpAndRunning', | |||
| COULD_NOT_SERIALIZ_EERROR: 'kCouldNotSerializeError', | |||
There was a problem hiding this comment.
I think the _ ended up in the wrong place? ;)
There was a problem hiding this comment.
Oops, my fault /w\
lib/internal/worker.js
Outdated
| STDIO_PAYLOAD: 'kStdioPayload', | ||
| STDIO_WANTS_MORE_DATA: 'kStdioWantsMoreData', | ||
| LOAD_SCRIPT: 'kLoadScript' | ||
| }; |
There was a problem hiding this comment.
What if we use variables instead of properties of an object? This way, mistakes/typos can be caught at linting time, and errors like the one above COULD_NOT_SERIALIZ_EERROR would be more difficult to make (one has to make it at least twice for it to pass linting).
There was a problem hiding this comment.
Using variables may not prevent typos like COULD_NOT_SERIALIZ_EERROR because most of developers are using auto complete when coding. Using properties of an object can make developer benefits from the auto complete (just type messageTypes. and these constants will be showed)
This change can prevent typos and redundant strings in code.
starkwang
force-pushed
the
worker-message-types
branch
from
91307e5 to
c502795
Compare
|
Landed in ebf5b58 |
This change can prevent typos and redundant strings in code. PR-URL: #21537 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This change can prevent typos and redundant strings in code. PR-URL: #21537 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This change can prevent typos and redundant strings in code.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes