Repository: typedorm/typedorm (ts)

Issue:
circular peer-dependency: core <-> common
Hi there,

great project. Thank you very much.

I noticed that both packages ("core" and "common") list each other as a peerDependency. I am using serverless-bundle (https://github.com/AnomalyInnovations/serverless-bundle) to package my lambda functions and I have to list both "core" and "common" as "externals" to get typedorm to work after packaging (not 100% sure why that is, webpack seems to strip some necessary parts). However the circular dependency causes the bundler to go into an endless loop trying installing core, common, core, common and so on.
I can manually remove the peer dependencies and get it to work, but this is of course not a real solution.
What do you think about this? I can provide more details if needed.

Cheers
Paul

Pull request:
fix(common): remove circular peer dep
closes #66 

Patch:
diff --git a/packages/common/package.json b/packages/common/package.json
index e5f41f2..02f3214 100644
--- a/packages/common/package.json
+++ b/packages/common/package.json
@@ -9,7 +9,6 @@
     "orm"
   ],
   "peerDependencies": {
-    "@typedorm/core": "0.0.0-PLACEHOLDER",
     "reflect-metadata": "^0.1.13"
   },
   "dependencies": {
diff --git a/packages/core/src/classes/expression/base-expression-input.ts b/packages/core/src/classes/expression/base-expression-input.ts
index 340a6ef..a559ce5 100644
--- a/packages/core/src/classes/expression/base-expression-input.ts
+++ b/packages/core/src/classes/expression/base-expression-input.ts
@@ -167,38 +167,9 @@ export abstract class BaseExpressionInput {
     inputs: T[],
     strategy: MERGE_STRATEGY
   ) {
-    // check if base expression has any value
-    if (this.expression) {
-      this.expression = `(${this.expression})`;
-      this.appendToExpression(strategy);
-    }
-
-    inputs.forEach((input, index) => {
-      this.appendToExpression(`(${input.expression})`);
-
-      if (index !== inputs.length - 1) {
-        this.appendToExpression(strategy);
-      }
-
-      Object.keys(input.names).forEach(nameKey => {
-        if (this.names[nameKey]) {
-          throw new Error(
-            `Failed to merge expression attribute names, there are multiple attributes names with key "${nameKey}"`
-          );
-        }
-      });
-      Object.keys(input.values).forEach(valueKey => {
-        if (this.names[valueKey]) {
-          throw new Error(
-            `Failed to merge expression attribute values, there are multiple attributes values with key "${valueKey}"`
-          );
-        }
-      });
-
-      this.names = {...this.names, ...input.names};
-      this.values = {...this.values, ...input.values};
+    inputs.forEach(input => {
+      this.merge(input, strategy);
     });
-
     return this;
   }
 


Test patch:
diff --git a/packages/core/src/classes/expression/__test__/base-expression-input.spec.ts b/packages/core/src/classes/expression/__test__/base-expression-input.spec.ts
index 13900b8..43a6bbf 100644
--- a/packages/core/src/classes/expression/__test__/base-expression-input.spec.ts
+++ b/packages/core/src/classes/expression/__test__/base-expression-input.spec.ts
@@ -185,7 +185,7 @@ test('merges conditions when merging condition is empty', () => {
  * @group mergeMany
  */
 
-test('merges many conditions with AND operator', () => {
+test('merges many conditions', () => {
   const merged = new TestCondition()
     .lessThan('age', 12)
     .mergeMany(
@@ -210,36 +210,7 @@ test('merges many conditions with AND operator', () => {
       ':TC_status': 'active',
     },
     expression:
-      '(#TC_age < :TC_age) AND (#TC_name = :TC_name) AND (#TC_status = :TC_status) AND (#TC_score > :TC_score)',
-  });
-});
-
-test('merges many conditions with OR operator', () => {
-  const merged = new TestCondition()
-    .lessThan('age', 12)
-    .mergeMany(
-      [
-        new TestCondition().equals('name', 'sid'),
-        new TestCondition().equals('status', 'active'),
-        new TestCondition().greaterThan('score', 120),
-      ],
-      MERGE_STRATEGY.OR
-    );
-  expect(merged).toEqual({
-    _names: {
-      '#TC_age': 'age',
-      '#TC_name': 'name',
-      '#TC_score': 'score',
-      '#TC_status': 'status',
-    },
-    _values: {
-      ':TC_age': 12,
-      ':TC_name': 'sid',
-      ':TC_score': 120,
-      ':TC_status': 'active',
-    },
-    expression:
-      '(#TC_age < :TC_age) OR (#TC_name = :TC_name) OR (#TC_status = :TC_status) OR (#TC_score > :TC_score)',
+      '(((#TC_age < :TC_age) AND (#TC_name = :TC_name)) AND (#TC_status = :TC_status)) AND (#TC_score > :TC_score)',
   });
 });
 
diff --git a/packages/core/src/classes/manager/__test__/entity-manager.spec.ts b/packages/core/src/classes/manager/__test__/entity-manager.spec.ts
index 4caf8f1..9872fe7 100644
--- a/packages/core/src/classes/manager/__test__/entity-manager.spec.ts
+++ b/packages/core/src/classes/manager/__test__/entity-manager.spec.ts
@@ -665,7 +665,7 @@ test('finds items matching given query params and options', async () => {
     KeyConditionExpression:
       '(#KY_CE_PK = :KY_CE_PK) AND (begins_with(#KY_CE_SK, :KY_CE_SK))',
     FilterExpression:
-      '(#FE_age BETWEEN :FE_age_start AND :FE_age_end) AND (#FE_name = :FE_name) AND (attribute_exists(#FE_status))',
+      '((#FE_age BETWEEN :FE_age_start AND :FE_age_end) AND (#FE_name = :FE_name)) AND (attribute_exists(#FE_status))',
     Limit: 10,
     ScanIndexForward: true,
     TableName: 'test-table',