refactor(microservices) refactor, improvements, use es6 map #1161

kamilmysliwiec · 2018-10-03T20:16:23Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[x] Code style update (formatting, local variables)
[x] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

[x] Yes
[ ] No

Other information

Breaking change due to the different API of custom transport strategies (microservices)

coveralls · 2018-10-03T20:25:08Z

Pull Request Test Coverage Report for Build 1317

69 of 71 (97.18%) changed or added relevant lines in 21 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.03%) to 93.775%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
packages/common/utils/http-exception-body.util.ts	3	4	75.0%
packages/microservices/server/server-nats.ts	6	7	85.71%

Totals
Change from base Build 1282:	-0.03%
Covered Lines:	2845
Relevant Lines:	2967

💛 - Coveralls

tonivj5 · 2018-10-03T21:39:06Z

packages/microservices/server/server-mqtt.ts

@@ -43,7 +43,7 @@ export class ServerMqtt extends Server implements CustomTransportStrategy {

  public bindEvents(mqttClient: MqttClient) {
    mqttClient.on(MESSAGE_EVENT, this.getMessageHandler(mqttClient).bind(this));
-    const registeredPatterns = Object.keys(this.messageHandlers);
+    const registeredPatterns = [...this.messageHandlers.keys()];


It could be simplified using this.messageHandlers.keys() instead of [...this.messageHandlers.keys()] 👍

Map.keys() returns an Iterator (not an array)

You're correct! Good catch 😆

tonivj5 · 2018-10-03T21:39:18Z

packages/microservices/server/server-nats.ts

@@ -40,7 +40,7 @@ export class ServerNats extends Server implements CustomTransportStrategy {
  }

  public bindEvents(client: Client) {
-    const registeredPatterns = Object.keys(this.messageHandlers);
+    const registeredPatterns = [...this.messageHandlers.keys()];


tonivj5 · 2018-10-03T21:39:27Z

packages/microservices/server/server-redis.ts

@@ -52,7 +52,7 @@ export class ServerRedis extends Server implements CustomTransportStrategy {

  public bindEvents(subClient: RedisClient, pubClient: RedisClient) {
    subClient.on(MESSAGE_EVENT, this.getMessageHandler(pubClient).bind(this));
-    const subscribePatterns = Object.keys(this.messageHandlers);
+    const subscribePatterns = [...this.messageHandlers.keys()];


tonivj5 · 2018-10-03T21:41:38Z

packages/microservices/server/server.ts

-  ): (data) => Promise<Observable<any>> | null {
-    return this.messageHandlers[pattern] ? this.messageHandlers[pattern] : null;
+  public getHandlerByPattern(pattern: string): MessageHandler | null {
+    return this.messageHandlers.has(pattern)


It's okey, but it could be shorter if you use this.messageHandlers.get(pattern) || null :-)

tonivj5 · 2018-10-03T21:43:16Z

packages/microservices/test/server/server-grpc.spec.ts

@@ -80,10 +80,12 @@ describe('ServerGrpc', () => {

  describe('createService', () => {
    it('should call "createServiceMethod"', async () => {
-      const handlers = {
+      const objectToMap = obj =>


This function is used in more places (copy/pasted), maybe it could be put in one place and reuse it

Good clean-code

…plified_if

…ied_if

refactor() simplified if (less checks)

…che/http)

feature(common) add validation error factory (ValidationPipe)

feature(common) add async support (HttpModule) and extraProviders

bugfix(ws/microservices): add catchError() to observables

bugfix(common) invalid metadata inheritance with custom providers

bugfix(common): custom exception body #1339

feature(microservices): added option for proto-loader (grpc)

chore() 5.5.0 minor release

lock · 2019-09-24T07:12:14Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

refactor(microservices) refactor, improvements, use es6 map

f802f94

kamilmysliwiec added scope: microservices type: code style labels Oct 3, 2018

tonivj5 reviewed Oct 3, 2018

View reviewed changes

kamilmysliwiec modified the milestones: 6.00, 6.0.0 Oct 5, 2018

jbpionnier and others added 18 commits October 18, 2018 11:52

refactor: simplified isEmpty

41190d7

refactor: simplified isFunction

ff3ef16

refactor: simplified validatePath

1f59ec3

refactor: simplified isObject

6db29c6

refactor: simplified isString

e1b8d3b

Merge branch 'master' of github.com:jbpionnier/nest into refactor_sim…

0d29cd7

…plified_if

feat(grpc): added option for proto-loader

1bd7048

feat(client-grpc): added protoLoader option for grpc client

4b55261

fixed protoLoader option

be40001

Merge branch 'master' of github.com:nestjs/nest into refactor_simplif…

3a29753

…ied_if

Merge pull request #1206 from jbpionnier/refactor_simplified_if

c6ec887

refactor() simplified if (less checks)

bugfix(common): custom exception body #1339

411db0c

bugfix(common) invalid metadata inheritance with custom providers

fe96bf6

bugfix(ws/microservices): add catchError() to observables

efb367e

feature(common) add async support (HttpModule) and extraProviders (ca…

e66d72f

…che/http)

fix(common) add missing interfaces reexport (http)

9fa12b4

feature(common) add validation error factory (ValidationPipe)

ee9d316

Merge pull request #1348 from nestjs/feature/1267-validation-factory

b632f60

feature(common) add validation error factory (ValidationPipe)

kamilmysliwiec added 10 commits December 6, 2018 19:44

Merge pull request #1347 from nestjs/feature/1257-http-async

ed37b91

feature(common) add async support (HttpModule) and extraProviders

Merge pull request #1346 from nestjs/bugfix/1285-throw-error

d25e6c7

bugfix(ws/microservices): add catchError() to observables

Merge pull request #1345 from nestjs/bugfix/1338-injection-scopes

63e3940

bugfix(common) invalid metadata inheritance with custom providers

Merge pull request #1344 from nestjs/bugfix/1339-exceptions

6a54900

bugfix(common): custom exception body #1339

Merge pull request #1271 from grin0c/another-proto-loader

8bce796

feature(microservices): added option for proto-loader (grpc)

Merge pull request #1349 from nestjs/5.5.0

17b0100

chore() 5.5.0 minor release

bugfix(common) inconsistent microservice config #1352

438e662

chore(@nestjs) publish v5.5.0 release

ef2147d

merge() resolve conflicts

30b6c50

chore() resolve conflicts

4796023

kamilmysliwiec changed the base branch from master to 6.0.0-next December 10, 2018 21:04

test(microservices) fix unit tests

4ec8e34

kamilmysliwiec merged commit f96b2a8 into 6.0.0-next Dec 10, 2018

delete-merged-branch bot deleted the refactor/microservices branch December 10, 2018 21:38

lock bot locked as resolved and limited conversation to collaborators Sep 24, 2019

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(microservices) refactor, improvements, use es6 map #1161

refactor(microservices) refactor, improvements, use es6 map #1161

kamilmysliwiec commented Oct 3, 2018 •

edited

Loading

coveralls commented Oct 3, 2018 •

edited

Loading

tonivj5 Oct 3, 2018

kamilmysliwiec Oct 4, 2018

tonivj5 Oct 4, 2018

tonivj5 Oct 3, 2018

kamilmysliwiec Oct 4, 2018

tonivj5 Oct 3, 2018

kamilmysliwiec Oct 4, 2018

tonivj5 Oct 3, 2018

Caballerog Oct 3, 2018

tonivj5 Oct 3, 2018

Caballerog Oct 3, 2018

lock bot commented Sep 24, 2019

refactor(microservices) refactor, improvements, use es6 map #1161

refactor(microservices) refactor, improvements, use es6 map #1161

Conversation

kamilmysliwiec commented Oct 3, 2018 • edited Loading

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

coveralls commented Oct 3, 2018 • edited Loading

Pull Request Test Coverage Report for Build 1317

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lock bot commented Sep 24, 2019

kamilmysliwiec commented Oct 3, 2018 •

edited

Loading

coveralls commented Oct 3, 2018 •

edited

Loading