Repository: ViacomInc/data-point (js)

Issue:
Use regex for splitting reducer strings
**Problem description**:

When creating reducers with a `|` separated string, you can't have more than one space on either side of the pipe:

```js
// this should return 1, but it errors instead
dataPoint.resolve('$a |    $b', { a: { b: 1 } })

// Unhandled rejection Error: Invalid reducer type. Could not find a matching reducer type while parsing the value:
//  ' $b'
// Try using an Array, String, Object, or Function.
```

**Suggested solution**:

This line should use a regex instead of a plain string:

https://github.com/ViacomInc/data-point/blob/master/packages/data-point/lib/reducer-types/reducer-list/factory.js#L36

```js
// replace this
reducerSource.split(' | ')

// with this
reducerSource.split(/\s+\|\s+/)
```

We can also trim the original string if we're not doing that already.

Pull request:
Use regex to split piped reducer-lists
<!--
Thanks for your interest in the project. We appreciate bugs filed and PRs submitted!

Please make sure that you are familiar with and follow the Code of Conduct for
this project (found in the CODE_OF_CONDUCT.md file).

Also, please make sure you're familiar with and follow the instructions in the
contributing guidelines (found in the CONTRIBUTING.md file).

Please fill out the information below to expedite the review and (hopefully)
merge of your pull request!

BREAKING CHANGES:

If your PR includes a breaking change, please submit it with a codemod under
data-point-codemods that will help users upgrade their codebase.

Breaking changes without a codemod will not be accepted unless a codemod is not
viable or does not apply to the specific situation.
-->

<!-- What changes are being made? (What feature/bug is being fixed here?) -->
**What**: Use regex to split piped reducer lists

<!-- Why are these changes necessary? -->
**Why**: Closes #227

<!-- How were these changes implemented? -->
**How**: Copied what @raingerber suggested in the issue

<!-- Have you done all of these things?  -->
**Checklist**:
<!-- add "N/A" to the end of each line that's irrelevant to your changes -->
<!-- to check an item, place an "x" in the box like so: "- [x] Documentation" -->
- [ ] Has Breaking changes
- [ ] Documentation
- [X] Tests
- [X] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? -->
- [ ] Added username to **all-contributors** list

<!-- feel free to add additional comments -->

I added two tests to check that the regex works with extra spaces surrounding the `|` or at the end of the string.

Patch:
diff --git a/packages/data-point/lib/reducer-types/reducer-list/factory.js b/packages/data-point/lib/reducer-types/reducer-list/factory.js
index 9e1954a..426eb09 100644
--- a/packages/data-point/lib/reducer-types/reducer-list/factory.js
+++ b/packages/data-point/lib/reducer-types/reducer-list/factory.js
@@ -32,8 +32,8 @@ module.exports.isType = isType
  * @returns {Array}
  */
 function parseFromString (source) {
-  const reducerSource = _.defaultTo(source, '')
-  const tokens = reducerSource.split(' | ')
+  const reducerSource = _.defaultTo(source, '').trim()
+  const tokens = reducerSource.split(/\s+\|\s+/)
   return tokens
 }
 


Test patch:
diff --git a/packages/data-point/lib/reducer-types/reducer-list/factory.test.js b/packages/data-point/lib/reducer-types/reducer-list/factory.test.js
index 0e682e0..dfbc768 100644
--- a/packages/data-point/lib/reducer-types/reducer-list/factory.test.js
+++ b/packages/data-point/lib/reducer-types/reducer-list/factory.test.js
@@ -37,6 +37,18 @@ describe('factory#create', () => {
     expect(result.reducers[2].type).toBe('ReducerFunction')
   })
 
+  test('it should create a ReducerList with reducers using a piped reducer with multiple spaces', () => {
+    const result = factory.create(createReducer, [
+      '$foo.bar  |     reducer:add   ',
+      () => true
+    ])
+
+    expect(result.reducers).toHaveLength(3)
+    expect(result.reducers[0].type).toBe('ReducerPath')
+    expect(result.reducers[1].type).toBe('ReducerEntity')
+    expect(result.reducers[2].type).toBe('ReducerFunction')
+  })
+
   test('It should create a ReducerList with reducers using an array of reducers', () => {
     const result = factory.create(createReducer, [
       '$foo.bar',
@@ -78,6 +90,16 @@ describe('factory#create', () => {
     expect(result.reducers[2].type).toBe('ReducerPath')
   })
 
+  test('factory#create reducer from grouped reducers with multiple leading/trailing spaces', () => {
+    const result = factory.create(createReducer, [
+      '     $foo    |    $bar    |     $baz    '
+    ])
+    expect(result.reducers).toHaveLength(3)
+    expect(result.reducers[0].type).toBe('ReducerPath')
+    expect(result.reducers[1].type).toBe('ReducerPath')
+    expect(result.reducers[2].type).toBe('ReducerPath')
+  })
+
   test('factory#create errors if invalid reducer type', () => {
     const result = _.attempt(factory.create, createReducer, 1)